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

Use FNV hash for Key #1736

Merged
merged 1 commit into from Jan 13, 2024
Merged

Use FNV hash for Key #1736

merged 1 commit into from Jan 13, 2024

Conversation

lahma
Copy link
Collaborator

@lahma lahma commented Jan 12, 2024

@@ -14,7 +15,7 @@ namespace Jint
private Key(string name)
{
Name = name;
HashCode = StringComparer.Ordinal.GetHashCode(name);
HashCode = Hash.GetFNVHashCode(name);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should it be lazy or is any Key always checked at least once for equality?

Copy link
Collaborator Author

@lahma lahma Jan 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The keys are basically always used against dictionaries, we are trying to find identifiers in context (variables, object fields), we climb up the environment util root (global) and each time pre-calculated hash is being used.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A stupid idea you will debunk for sure: Assuming most keys are smallish, can't we use an optimization by using the raw memory value of the strings < 8 chars (64bits -> int) as the hashcode? And then fallback above 8 chars? This way for <8 chars we just need to compare the hashes too, and the computation could be faster (TBD).

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a stupid idea for sure, makes sense but I'm not an entropy expert (how that would affect collisions).

I also played with the idea of having length < 4 directly interned and then reference equals would suffice in Key context.

I hope to create a new baseline to improve upon, but feels that perf runs can be quite unreliable - results vary a lot...

@lahma
Copy link
Collaborator Author

lahma commented Jan 12, 2024

Jint.Benchmark.DromaeoBenchmark

Diff Method Toolchain FileName Mean Error Allocated
Old Run Default dromaeo-3d-cube 22.934 ms 0.0874 ms 6284.28 KB
New Default 22.781 ms (-1%) 0.0802 ms 6284.28 KB (0%)
Old Run Default dromaeo-core-eval 4.567 ms 0.0131 ms 311.47 KB
New Default 4.203 ms (-8%) 0.0106 ms 311.47 KB (0%)
Old Run Default dromaeo-object-array 45.107 ms 0.2320 ms 100362.96 KB
New Default 44.774 ms (-1%) 0.0921 ms 100362.95 KB (0%)
Old Run Default droma(...)egexp [21] 164.136 ms 1.2785 ms 160554.4 KB
New Default 161.919 ms (-1%) 1.8212 ms 165388.61 KB (+3%)
Old Run Default droma(...)tring [21] 305.667 ms 8.9598 ms 1321524.27 KB
New Default 327.253 ms (+7%) 9.7981 ms 1321594.6 KB (0%)
Old Run Default droma(...)ase64 [21] 52.778 ms 0.1894 ms 6045.2 KB
New Default 47.342 ms (-10%) 0.1317 ms 6045.71 KB (0%)
Old Run Default dromaeo-3d-cube 21.214 ms 0.0316 ms 5994.93 KB
New Default 20.918 ms (-1%) 0.0508 ms 5994.94 KB (0%)
Old Run Default dromaeo-core-eval 4.246 ms 0.0173 ms 298.91 KB
New Default 3.938 ms (-7%) 0.0345 ms 298.91 KB (0%)
Old Run Default dromaeo-object-array 45.387 ms 0.1600 ms 100324.03 KB
New Default 43.991 ms (-3%) 0.2207 ms 100324.05 KB (0%)
Old Run Default droma(...)egexp [21] 155.237 ms 1.6311 ms 163082.43 KB
New Default 156.562 ms (+1%) 2.4694 ms 168520.57 KB (+3%)
Old Run Default droma(...)tring [21] 316.258 ms 10.3567 ms 1321256.9 KB
New Default 320.153 ms (+1%) 14.0838 ms 1321238.15 KB (0%)
Old Run Default droma(...)ase64 [21] 50.761 ms 0.1518 ms 5957.75 KB
New Default 48.152 ms (-5%) 0.1664 ms 5958.38 KB (0%)

@lahma
Copy link
Collaborator Author

lahma commented Jan 13, 2024

Method Input Mean Error StdDev Ratio RatioSD Rank
Fnv1 encodeURIComponent 8.2694 ns 0.0245 ns 0.0229 ns 0.80 0.00 1
StringHashCode encodeURIComponent 10.3244 ns 0.0579 ns 0.0513 ns 1.00 0.00 2
StringOrdinalHashCode encodeURIComponent 10.7936 ns 0.1451 ns 0.1357 ns 1.04 0.01 3
Hash3 encodeURIComponent 13.9756 ns 0.0278 ns 0.0260 ns 1.35 0.01 4
Fnv1 i 0.1548 ns 0.0030 ns 0.0028 ns 0.09 0.00 1
StringHashCode i 1.6543 ns 0.0003 ns 0.0003 ns 1.00 0.00 2
StringOrdinalHashCode i 1.9382 ns 0.0093 ns 0.0083 ns 1.17 0.00 3
Hash3 i 9.7149 ns 0.0242 ns 0.0227 ns 5.87 0.01 4
Fnv1 Math 1.2434 ns 0.0055 ns 0.0051 ns 0.39 0.00 1
StringHashCode Math 3.1640 ns 0.0005 ns 0.0005 ns 1.00 0.00 2
StringOrdinalHashCode Math 3.3992 ns 0.0093 ns 0.0087 ns 1.07 0.00 3
Hash3 Math 9.9248 ns 0.0015 ns 0.0014 ns 3.14 0.00 4
Fnv1 str 0.5622 ns 0.0058 ns 0.0054 ns 0.23 0.00 1
StringHashCode str 2.4333 ns 0.0080 ns 0.0075 ns 1.00 0.00 2
StringOrdinalHashCode str 2.4922 ns 0.0073 ns 0.0068 ns 1.02 0.00 3
Hash3 str 9.7433 ns 0.0375 ns 0.0351 ns 4.00 0.02 4

@lahma lahma merged commit 8047b02 into sebastienros:main Jan 13, 2024
3 checks passed
@lahma lahma deleted the fnv-hash branch January 13, 2024 09:11
scgm0 pushed a commit to scgm0/jint that referenced this pull request Jan 25, 2024
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.

None yet

2 participants