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.

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

Replace sse4_crc32 with @node-rs/crc32 #21

Open
Brooooooklyn opened this issue Apr 30, 2020 · 9 comments · May be fixed by #22
Open

Replace sse4_crc32 with @node-rs/crc32 #21

Brooooooklyn opened this issue Apr 30, 2020 · 9 comments · May be fixed by #22

Comments

@Brooooooklyn
Copy link

Brooooooklyn commented Apr 30, 2020 •
edited
Loading

https://github.com/Brooooooklyn/node-rs/tree/master/packages/crc32

Here is benchmark on my machine:

Model Name: MacBook Pro
Model Identifier: MacBookPro15,1
Processor Name: Intel Core i7
Processor Speed: 2.6 GHz
Number of Processors: 1
Total Number of Cores: 6
L2 Cache (per Core): 256 KB
L3 Cache: 12 MB
Hyper-Threading Technology: Enabled
Memory: 16 GB

Benchmark:

@node-rs/crc32c for inputs 1024B x 5,264,095 ops/sec ±1.20% (93 runs sampled)
@node-rs/crc32c for inputs 16829745B, avg 2054B x 263 ops/sec ±11.01% (71 runs sampled)
sse4_crc32c_hw for inputs 1024B x 3,605,658 ops/sec ±1.00% (91 runs sampled)
sse4_crc32c_hw for inputs 16829745B, avg 2054B x 211 ops/sec ±1.08% (72 runs sampled)
sse4_crc32c_sw for inputs 1024B x 1,533,277 ops/sec ±0.83% (92 runs sampled)
sse4_crc32c_sw for inputs 16829745B, avg 2054B x 99.14 ops/sec ±1.90% (73 runs sampled)
js_crc32c for inputs 1024B x 439,854 ops/sec ±2.55% (89 runs sampled)
js_crc32c for inputs 16829745B, avg 2054B x 26.27 ops/sec ±2.87% (47 runs sampled)
js_crc32 for inputs 1024B x 422,623 ops/sec ±2.52% (83 runs sampled)
js_crc32 for inputs 16829745B, avg 2054B x 28.34 ops/sec ±1.64% (51 runs sampled)
+-----------------+-------------------+----------------------+
|                 │ 1024B             │ 16829745B, avg 2054B |
+-----------------+-------------------+----------------------+
| @node-rs/crc32c │ 5,264,095 ops/sec │ 263 ops/sec          |
+-----------------+-------------------+----------------------+
| sse4_crc32c_hw  │ 3,605,658 ops/sec │ 211 ops/sec          |
+-----------------+-------------------+----------------------+
| sse4_crc32c_sw  │ 1,533,277 ops/sec │ 99.14 ops/sec        |
+-----------------+-------------------+----------------------+
| js_crc32c       │ 439,854 ops/sec   │ 26.27 ops/sec        |
+-----------------+-------------------+----------------------+
| js_crc32        │ 422,623 ops/sec   │ 28.34 ops/sec        |
+-----------------+-------------------+----------------------+
@Brooooooklyn Brooooooklyn changed the title Replace sse4_crc32 to @node-rs/crc32 Replace sse4_crc32 with @node-rs/crc32 Apr 30, 2020
@ashi009
Copy link
Owner

ashi009 commented Apr 30, 2020

This is interesting. By implementing CRC32C in rust alone can make such a performance difference?

@Brooooooklyn
Copy link
Author

Seems like not only the differences between the languages, but also the implementation inside.

@Brooooooklyn Brooooooklyn linked a pull request May 6, 2020 that will close this issue
Open
@kiyui
Copy link

kiyui commented Jan 8, 2021

Bumping this as sse4_crc32 is not working on darwin-arm64 at the moment 😅

> require('@node-rs/crc32')
{ crc32: [Function: crc32], crc32c: [Function: crc32c] }
> require('sse4_crc32')
dyld: lazy symbol binding failed: Symbol not found: __Z18sse42_is_availablev
  Referenced from: /Users/dafne/Workspaces/Piktochart/piktostory/node_modules/sse4_crc32/build/Release/crc32c.node
  Expected in: flat namespace

dyld: Symbol not found: __Z18sse42_is_availablev
  Referenced from: /Users/dafne/Workspaces/Piktochart/piktostory/node_modules/sse4_crc32/build/Release/crc32c.node
  Expected in: flat namespace

@Brooooooklyn
Copy link
Author

@kiyui this PR is staled.. Run yarn add @node-rs/crc32 to install latest version and it support darwin-arm64 natively

@kiyui
Copy link

kiyui commented Jan 8, 2021

The @google-cloud/storage docs recommend installing fast-crc32c to speed up crc32c computation. In turn, @google-cloud/storage requires hash-stream-validation which in turn requires this library.

I would not be bumping this issue if not for this coupling.

@louisgv louisgv mentioned this issue Apr 16, 2021
Closed
buu700 added a commit to buu700/node-fast-crc32c that referenced this issue May 5, 2021
@buu700
fix and optimization for non-x86 architectures
0e11a97 
ashi009#21
@buu700 buu700 mentioned this issue May 5, 2021
Open
@mpetrunic mpetrunic mentioned this issue May 26, 2021
Merged
@abhiaiyer91
Copy link

Hi, just revisiting this issue. Since google node-storage depends on this module can we get the upstream changes of @chainsafe/fast-crc32c and published?

@johngrogg
Copy link

Bumping this ask. We're using fast-crc32c in our monorepo, and until this is resolved the entire repo fails to run on M1 macs since the current sse4_crc32 dependency causes a node crash during package import.

@madebymichaelg
Copy link

Hey y'all, also wanted to jump in and mention this is a blocker in my organization. If we could merge #28 that would be great!

@jimmcslim jimmcslim mentioned this issue May 11, 2023
Closed
@jimmcslim
Copy link

Hello everybody! If you have landed here because you are trying to use @google-cloud/storage on an Apple Silicon Mac, I have raised an issue on the GitHub repo for this library... linked above. Hopefully Google updates their library to remove this dependency!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

perf: replace sse4_crc32 to @node-rs/crc32 Brooooooklyn/node-fast-crc32c
7 participants
@johngrogg @jimmcslim @ashi009 @abhiaiyer91 @madebymichaelg @Brooooooklyn @kiyui