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

PERF: Consider using xxhash #5297

Closed
4 tasks done
LifeIsStrange opened this issue Oct 14, 2021 · 7 comments
Closed
4 tasks done

PERF: Consider using xxhash #5297

LifeIsStrange opened this issue Oct 14, 2021 · 7 comments

Comments

@LifeIsStrange
Copy link

LifeIsStrange commented Oct 14, 2021

Clear and concise description of the problem

Hi @yyx990803

xxhash3 is the most popular next generation hashing algorithm although there are many other contenders.
A significantly faster hashing algorithm could reduce bottlenecks?

Suggested solution

Webpack is switching to xxhash64 by default
webpack/webpack#14306
Esbuild switched to xxhash64
evanw/esbuild#1099
evanw/esbuild#1107
(which is slower than xxhash3)

No idea about what rollup use.

Since this can lead to potentially significant performance win, I suggest Vite to migrate to xxhash64 or the faster xxhash3.

BTW xxhash60 (no idea about xxhash3) would make the hashing FIPS compliant
webpack/webpack#13572

Question:
Does vite hashes needs to be aligned with the same algorithm than esbuild/rollup? No idea
The length of hashes should no be altered Iengthened guess? It would be a visual regression for debugging/readability if hashes were longer.

Alternative

xxhash is great but alternatives could be considered, especially if xxhash3 is not enough to eliminate all hash related bottlenecks.

Wyhash is significantly faster then xxhash3 in general.
https://github.com/tkaitchuck/aHash is significantly faster than wyhash and is probably the fastest useful hashing function.

Additional context

.

Validations

@LifeIsStrange
Copy link
Author

Btw I wonder whether vite has a riche caching system similar to 'contenthash' on webpack
https://webpack.js.org/guides/caching/

@patak-dev
Copy link
Member

Interesting idea, I think it would be good to see a PR and some benchmarks after this change 👍🏼

@LifeIsStrange
Copy link
Author

I will not submit a PR myself unfortunately :)

@wangyi-fudan
Copy link

Clear and concise description of the problem

Hi @yyx990803

xxhash3 is the most popular next generation hashing algorithm although there are many other contenders. A significantly faster hashing algorithm could reduce bottlenecks?

Suggested solution

Webpack is switching to xxhash64 by default webpack/webpack#14306 Esbuild switched to xxhash64 evanw/esbuild#1099 evanw/esbuild#1107 (which is slower than xxhash3)

No idea about what rollup use.

Since this can lead to potentially significant performance win, I suggest Vite to migrate to xxhash64 or the faster xxhash3.

BTW xxhash60 (no idea about xxhash3) would make the hashing FIPS compliant webpack/webpack#13572

Question: Does vite hashes needs to be aligned with the same algorithm than esbuild/rollup? No idea The length of hashes should no be altered Iengthened guess? It would be a visual regression for debugging/readability if hashes were longer.

Alternative

xxhash is great but alternatives could be considered, especially if xxhash3 is not enough to eliminate all hash related bottlenecks.

Wyhash is significantly faster then xxhash3 in general. https://github.com/tkaitchuck/aHash is significantly faster than wyhash and is probably the fastest useful hashing function.

Additional context

.

Validations

ahash is slower than wyhash despite ahash's own claim in rust. smhasher has both benchmarks.

@LifeIsStrange
Copy link
Author

Yeah you're probably right
https://github.com/rurban/smhasher

@bluwy
Copy link
Member

bluwy commented Mar 28, 2022

The only hashing library I see Vite is using is hash-sum, and that's for plugin-vue only. Otherwise Vite itself implements a simple hash function like so:

export function getHash(text: string): string {
return createHash('sha256').update(text).digest('hex').substring(0, 8)
}

And IMO I'm not sure bringing in a new library here is worth the effort, considering it would also increase the bundle size.

@bluwy bluwy mentioned this issue May 1, 2022
9 tasks
@patak-dev
Copy link
Member

Closing as #7975 has been merged

@github-actions github-actions bot locked and limited conversation to collaborators May 31, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants