alexgorbatchev / crc

Blazingly fast CRC implementations for node.js and browser
MIT License
349 stars 72 forks source link

Slowness #24

Closed dougwilson closed 10 years ago

dougwilson commented 10 years ago

So I'm making this here before I forget about it and stuff. I was going to switch express and everything to use this module, but it turns out crc.crc32 is way slower than require('buffer-crc32').unsigned for whatever reason. It may be worth looking into why that is :)

alexgorbatchev commented 10 years ago

interesting... this code was ported pretty much one for one from ruby implementation, which isn't a reason why it should be fast of course ;) I'd love to get a hand figuring this out. When you say "way slower" what do you mean?

dougwilson commented 10 years ago

This commit pretty much sums it up: https://github.com/jshttp/etag/commit/5b9f83f8cf3f3cffbb8668d70c2252069850e70b

I was using require('crc').crc32 function before. Passing a 100 byte Buffer to it produces a crc32 value at a rate of 348,134 ops/sec, yet require('buffer-crc32').unsigned function did the same operation at a rate of 1,154,033 ops/sec.

Here is the full breakdown:

input require('crc').crc32(val) require('buffer-crc32').unsigned(val).toString(16)
100b Buffer 348,134 ops/sec 1,154,033 ops/sec
1kb Buffer 56,992 ops/sec 300,585 ops/sec
10kb Buffer 6,606 ops/sec 36,124 ops/sec
100kb Buffer 594 ops/sec 3,719 ops/sec
1mb Buffer 14 ops/sec 365 ops/sec

buffer

string

alexgorbatchev commented 10 years ago

That's total :poop:

alexgorbatchev commented 10 years ago

So the big difference between implementations is that my module returns hex string and buffer-crc32 returns a number... I think most of the perf penalty comes from hex conversion... how are you consuming the crc32 sum? are you expecting a hex or a number?

dougwilson commented 10 years ago

For this module, I'm calling require('crc').crc32(str). For buffer-crc32, I'm calling require('buffer-crc32').unsigned(str).toString(16) because I do want a hex string. The extra toString call for buffer-crc32 is included in those benchmarks.

Edit: I updated the table above to show the exact calls I'm making.

dougwilson commented 10 years ago

The extra toString call for buffer-crc32 is included in those benchmarks.

Which means buffer-crc32 is still faster, even though I'm making an extra call to get my hex string.

alexgorbatchev commented 10 years ago

I'm not getting buffer-crc32 numbers high as yours, but I just rewrote with some optimizations in mind, could you check against this branch please?

https://github.com/alexgorbatchev/node-crc/tree/optimization

also, which tool do you use to test & generate reports?

dougwilson commented 10 years ago

I'm not getting buffer-crc32 numbers high as yours

The numbers as absolute values are, of course, machine-dependent :) It's more about the percentage difference between the two libs.

also, which tool do you use to test & generate reports?

I'm just using the benchmarks in the etag library I added: https://github.com/jshttp/etag/tree/master/benchmark , though you cannot use them directly--you just need to edit the source to make it do crc32 only

could you check against this branch please?

yep. i'll report back in a little bit :)

dougwilson commented 10 years ago

Here is the results for the Buffer (the higher performing of the two; string is basically the same, but slightly slower for every module anyway). The performance for this module is vastly improved with your change:

crc-buf

alexgorbatchev commented 10 years ago

Nice, I just pushed another update... According to local benchmarks this module is now as fast or faster than buffer-crc32 in all cases :) Check it out

dougwilson commented 10 years ago

According to local benchmarks this module is now as fast or faster than buffer-crc32 in all cases :) Check it out

Indeed! This is awesome! Thanks for all the work :) (I don't know CoffeeScript, myself). I'm also sure all the library using this library will appreciate it :)

crc-buf2

alexgorbatchev commented 10 years ago

Fantastic! I published v3