search

nim-lang / Nim

Nim is a statically typed compiled systems programming language. It combines successful concepts from mature languages like Python, Ada and Modula. Its design focuses on efficiency, expressiveness, and elegance (in that order of priority).
https://nim-lang.org
Other
16.22k stars 1.46k forks source link

Add Farm Hash conditioned upon `nimPreviewHashFarm` as 64-bit `Hash` #23735

Closed c-blake closed 1 week ago

c-blake commented 1 week ago

Unlike present Nim this actually fills Hash for string & related.

For the curious, note that hashData remains the aboriginal Nim string hasher & import hashes {.all.} allows simultaneous test/time of {orig, murmur, farm} on your favorite CPU & back end compiler.

Update tests also conditioned upon nimPreviewHashFarm so they should pass either with or without that define on.

In --jsbigint=on mode, only the lower 32-bits of Hash match nimvm & run-time values because type Hash = int and on JS int=int32, not int64 as for 64-bit Nim platforms. Due to the matching, const Table should match run-time Table on all platforms.

To operate in --jsbigint=off mode is feasible but needs much "double precision mul/xor/ror/shr-arithmetic"-style work. That is distracting & also of questionable value since JS added BigInt in 2018, ringabout added Nim support for it in 2021 & nimPreviewHashFarm is unlikely to swap from an opt-in to an opt-out default before 2025..2026 which will have given a backward looking time window of 7..8 years for deployment platforms - reasonably generous.

Add a changelog entry for 2.2.

c-blake commented 1 week ago

Cross ref to https://github.com/nim-lang/Nim/issues/23678 discussion.

c-blake commented 1 week ago

FWIW, the failing tests have nothing to do with this PR (although perhaps the non-deterministic 7 second timeout in tests/vm/tslow_tables.nim should be increased a bit) since everything here is hidden behind a nimPreviewHashFarm.

juancarlospaco commented 1 week ago

Should jsbigint=off be Deprecated? 🤔

c-blake commented 1 week ago

Personally, I think jsbigint=off mode could be deprecated maybe in the same Nim-2.2 release time frame as this opt-in ability lands. That definitely has broader impact than just this PR and so should probably be a separate one.

Araq commented 1 week ago

We could even make it the default for 2.2 as long as it works with JavaScript out of the box too. (We need to make --jsbigint=on the default.)

github-actions[bot] commented 1 week ago

Thanks for your hard work on this PR! The lines below are statistics of the Nim compiler built from e64512036289434c6a7b377f52a50189beae75e8

Hint: mm: orc; opt: speed; options: -d:release 179006 lines; 8.665s; 664.305MiB peakmem

ringabout commented 1 week ago

We need to make --jsbigint=on the default.

--jsbigint=on has already been the default.

The tests need to be enabled with -d:nimPreviewHashFarm in the following PRs