New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Reduce fdiv's into fmul's #1204

Merged

copybara-service merged 5 commits into google:master from heshpdx:master

Nov 19, 2024

Contributor

heshpdx commented Oct 5, 2024

Provides small speedup on microarchitectures where the floating point divide is slower than the floating point multiply.

As measured on an Ampere Altra, I see about +0.5% depending on the input.

$ ./brotli.old -fvk -w 0 enwik8.xhtml
Compressed [enwik8.xhtml]: 95.367 MiB -> 24.549 MiB in 237.92 sec

$ ./brotli.new -fvk -w 0 enwik8.xhtml
Compressed [enwik8.xhtml]: 95.367 MiB -> 24.549 MiB in 236.72 sec

heshpdx commented

View reviewed changes

c/enc/block_splitter_inc.h Outdated Show resolved Hide resolved

Collaborator

eustas commented Nov 12, 2024

Awesome. Please add static constant for constants that derived from 1 / 2000.0; even better would be extracting 2000 itself as well to something like prologue_length, or simply threshold. Thanks.

eustas previously approved these changes

View reviewed changes

eustas force-pushed the master branch from 6d2cce7 to d6b943b Compare

November 12, 2024 15:37

heshpdx dismissed eustas’s stale review via

53ba414

November 12, 2024 16:25

Contributor Author

heshpdx commented Nov 12, 2024

Awesome. Please add static constant for constants that derived from 1 / 2000.0; even better would be extracting 2000 itself as well to something like prologue_length, or simply threshold. Thanks.

Done! All tests still pass.

eustas reviewed

View reviewed changes

c/enc/block_splitter_inc.h Outdated Show resolved Hide resolved

eustas reviewed

View reviewed changes

c/enc/block_splitter_inc.h Outdated Show resolved Hide resolved

eustas reviewed

View reviewed changes

c/enc/block_splitter_inc.h Outdated Show resolved Hide resolved

eustas reviewed

View reviewed changes

c/enc/literal_cost.c Outdated Show resolved Hide resolved

eustas reviewed

View reviewed changes

c/enc/literal_cost.c Outdated Show resolved Hide resolved

eustas reviewed

View reviewed changes

c/enc/literal_cost.c Outdated Show resolved Hide resolved

eustas previously approved these changes

View reviewed changes

Collaborator

eustas left a comment

Looks good. Added a bit of polishing with suggested-edits.

heshpdx dismissed eustas’s stale review via

d8bb37b

November 12, 2024 22:45

eustas previously approved these changes

View reviewed changes

eustas added the ready to pull label

eustas reviewed

View reviewed changes

c/enc/encode.c Outdated Show resolved Hide resolved

heshpdx dismissed eustas’s stale review via

5ff2357

November 15, 2024 18:39

eustas approved these changes

View reviewed changes

heshpdx and others added 5 commits

November 19, 2024 09:52


          Reduce fdiv's into fmul's

cefec3c

Provides small speedup on microarchitectures where the floating
point divide is slower than the floating point multiply.


          Update c/enc/block_splitter_inc.h

cec846f

Added a digit of precision


          Add static variables as per code review comments.

1054ecc


          Apply suggestions from code review

782aadd

Co-authored-by: Eugene Kliuchnikov <eustas@google.com>


          Update c/enc/encode.c

8c6d25f

Co-authored-by: Eugene Kliuchnikov <eustas@google.com>

eustas force-pushed the master branch from 5ff2357 to 8c6d25f Compare

November 19, 2024 08:52

copybara-service bot merged commit 2b6efcb into google:master

35 of 39 checks passed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels