Skip to content
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.

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

Improve performance by removing fnv dependency #21

Draft
wants to merge 2 commits into
base: mini
Choose a base branch
from

Conversation

aehancoc
Copy link

@aehancoc aehancoc commented Apr 13, 2024

Hey, long time no see. Thanks for maintaining this in my absence. (In case you're wondering, my old account was tied to my university email, which I lost access to.)

Anywho, I chose fnv back in b93b80f because std::collections::HashMap had somewhat worse performance. It seems things have changed, and now std's hashmap/hashset is better now. fnv is also really terrible with larger keys, which we're dealing with here due to using strings to store MIME types. I also took the liberty of adding the new test files to the benchmarks and removing the duplicate files there.

Some numbers from my Intel Core i5-1135G7:

fnv

     Running benches\from_u8.rs (target\release\deps\from_u8-ff4f06eaaf1dbfd9.exe)

running 18 tests
test application_x_7z_compressed ... bench:       3,103 ns/iter (+/- 409)
test application_x_tar           ... bench:      23,813 ns/iter (+/- 6,375)
test application_zip             ... bench:       6,430 ns/iter (+/- 1,344)
test audio_flac                  ... bench:      14,739 ns/iter (+/- 445)
test audio_mpeg                  ... bench:      21,131 ns/iter (+/- 3,454)
test audio_ogg                   ... bench:       6,906 ns/iter (+/- 337)
test audio_opus                  ... bench:       7,053 ns/iter (+/- 965)
test audio_wav                   ... bench:       7,615 ns/iter (+/- 818)
test image_bmp                   ... bench:      12,934 ns/iter (+/- 3,783)
test image_gif                   ... bench:       2,315 ns/iter (+/- 662)
test image_png                   ... bench:       2,448 ns/iter (+/- 1,183)
test image_tiff                  ... bench:      20,376 ns/iter (+/- 1,384)
test image_x_pcx                 ... bench:      13,892 ns/iter (+/- 3,323)
test image_x_portable_bitmap     ... bench:      14,249 ns/iter (+/- 3,162)
test image_x_tga                 ... bench:      10,709 ns/iter (+/- 1,521)
test image_xbm                   ... bench:      32,445 ns/iter (+/- 6,046)
test text_html                   ... bench:      29,509 ns/iter (+/- 4,487)
test text_plain                  ... bench:      28,937 ns/iter (+/- 6,604)

test result: ok. 0 passed; 0 failed; 0 ignored; 18 measured

     Running benches\match_u8.rs (target\release\deps\match_u8-bea7a4ade2ef60c5.exe)

running 18 tests
test application_x_7z_compressed ... bench:          84 ns/iter (+/- 17)
test application_x_tar           ... bench:          64 ns/iter (+/- 24)
test application_zip             ... bench:          60 ns/iter (+/- 13)
test audio_flac                  ... bench:          44 ns/iter (+/- 9)
test audio_mpeg                  ... bench:          52 ns/iter (+/- 4)
test audio_ogg                   ... bench:          41 ns/iter (+/- 6)
test audio_opus                  ... bench:          18 ns/iter (+/- 9)
test audio_wav                   ... bench:          58 ns/iter (+/- 9)
test image_bmp                   ... bench:          54 ns/iter (+/- 15)
test image_gif                   ... bench:          39 ns/iter (+/- 27)
test image_png                   ... bench:          46 ns/iter (+/- 5)
test image_tiff                  ... bench:          56 ns/iter (+/- 5)
test image_x_pcx                 ... bench:          91 ns/iter (+/- 35)
test image_x_portable_bitmap     ... bench:         134 ns/iter (+/- 117)
test image_x_tga                 ... bench:          61 ns/iter (+/- 100)
test image_xbm                   ... bench:          23 ns/iter (+/- 15)
test text_html                   ... bench:          90 ns/iter (+/- 32)
test text_plain                  ... bench:          36 ns/iter (+/- 21)

test result: ok. 0 passed; 0 failed; 0 ignored; 18 measured

std::collections

     Running benches\from_u8.rs (target\release\deps\from_u8-738934f266f65ebd.exe)

running 18 tests
test application_x_7z_compressed ... bench:      13,210 ns/iter (+/- 1,699)
test application_x_tar           ... bench:      20,664 ns/iter (+/- 4,171)
test application_zip             ... bench:       5,496 ns/iter (+/- 1,352)
test audio_flac                  ... bench:       9,246 ns/iter (+/- 5,188)
test audio_mpeg                  ... bench:       9,043 ns/iter (+/- 3,767)
test audio_ogg                   ... bench:       4,578 ns/iter (+/- 5,994)
test audio_opus                  ... bench:       4,980 ns/iter (+/- 2,177)
test audio_wav                   ... bench:       7,923 ns/iter (+/- 4,984)
test image_bmp                   ... bench:       8,892 ns/iter (+/- 4,375)
test image_gif                   ... bench:       2,219 ns/iter (+/- 742)
test image_png                   ... bench:       2,192 ns/iter (+/- 810)
test image_tiff                  ... bench:      10,511 ns/iter (+/- 6,045)
test image_x_pcx                 ... bench:       4,581 ns/iter (+/- 1,867)
test image_x_portable_bitmap     ... bench:       6,950 ns/iter (+/- 5,136)
test image_x_tga                 ... bench:      20,234 ns/iter (+/- 6,251)
test image_xbm                   ... bench:      40,380 ns/iter (+/- 16,540)
test text_html                   ... bench:      30,258 ns/iter (+/- 9,096)
test text_plain                  ... bench:      32,097 ns/iter (+/- 10,740)

test result: ok. 0 passed; 0 failed; 0 ignored; 18 measured

     Running benches\match_u8.rs (target\release\deps\match_u8-1b76e162075bf416.exe)

running 18 tests
test application_x_7z_compressed ... bench:          79 ns/iter (+/- 15)
test application_x_tar           ... bench:          81 ns/iter (+/- 31)
test application_zip             ... bench:          87 ns/iter (+/- 52)
test audio_flac                  ... bench:          74 ns/iter (+/- 37)
test audio_mpeg                  ... bench:          64 ns/iter (+/- 55)
test audio_ogg                   ... bench:          59 ns/iter (+/- 34)
test audio_opus                  ... bench:          35 ns/iter (+/- 35)
test audio_wav                   ... bench:          76 ns/iter (+/- 38)
test image_bmp                   ... bench:          82 ns/iter (+/- 34)
test image_gif                   ... bench:          71 ns/iter (+/- 25)
test image_png                   ... bench:          77 ns/iter (+/- 24)
test image_tiff                  ... bench:          87 ns/iter (+/- 60)
test image_x_pcx                 ... bench:          94 ns/iter (+/- 127)
test image_x_portable_bitmap     ... bench:         121 ns/iter (+/- 41)
test image_x_tga                 ... bench:          74 ns/iter (+/- 42)
test image_xbm                   ... bench:          32 ns/iter (+/- 28)
test text_html                   ... bench:          88 ns/iter (+/- 47)
test text_plain                  ... bench:          39 ns/iter (+/- 45)

test result: ok. 0 passed; 0 failed; 0 ignored; 18 measured

It's not a substantial improvement, but it's something, and it lets us drop a dependency. Feel free to test on your machine. Keep in mind that individual test performance for from_u8 is dependent in part on whatever order the graph nodes get inserted in.

This results in performance improvements in the benchmarks, surprisingly enough. Seems the default HashMap/HashSet got better over the past few years.
@aehancoc
Copy link
Author

I'm realizing the benchmarks might be easier to read if the benchmarks are ported to Criterion. There's almost certainly a good bit of jitter from the initial lazy load.

@mbrubeck
Copy link
Owner

Hey, long time no see. Thanks for maintaining this in my absence. (In case you're wondering, my old account was tied to my university email, which I lost access to.)

You’re welcome! And if you’re interested at any point in being the maintainer (or a co-maintainter) of this package again, just let me know. I don’t actually use this library in my other projects anymore, though I’m happy to continue keeping it up to date for others.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants