improve performance

[Uzlopak]

Hi @lukeed ,

with this PR we generate a id-function on the fly. There are also various changes like avoiding substring calls and initializing the HEX-Lookup-Table with index 0.

before:

Benchmark (length = 16):
  hexoid               x 189,320,225 ops/sec ±0.34% (95 runs sampled)

Benchmark (length = 25):
  hexoid               x 175,288,431 ops/sec ±0.58% (96 runs sampled)

Benchmark (length = 36):
  hexoid               x 163,136,858 ops/sec ±0.54% (97 runs sampled)

after:

Benchmark (length = 16):
  hexoid               x 382,579,366 ops/sec ±0.64% (97 runs sampled)

Benchmark (length = 25):
  hexoid               x 342,972,348 ops/sec ±0.81% (97 runs sampled)

Benchmark (length = 36):
  hexoid               x 310,474,341 ops/sec ±0.77% (90 runs sampled)

Benchmarks made on node v14.19.1

@mcollina
@kibertoad

[lukeed]

Hi there, thanks for the PR. Can you add all this to a new “src/unsafe.js” file? Because this uses new Function this isn’t usable in runtimes like Cloudflare Workers

[codecov-commenter]

Codecov Report

Merging #2 (7660ae8) into master (cdbae71) will not change coverage.
The diff coverage is 100.00%.

❗ Current head 7660ae8 differs from pull request most recent head 0d03acb. Consider uploading reports for the commit 0d03acb to get more accurate results

@@            Coverage Diff            @@
##            master        #2   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            1         2    +1     
  Lines           10        16    +6     
=========================================
+ Hits            10        16    +6     

Impacted Files	Coverage Δ
src/index.js	100.00% <100.00%> (ø)
src/unsafe.js	100.00% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cdbae71...0d03acb. Read the comment docs.

[Uzlopak]

@lukeed
I am kind of unfamiliar with modul-javascript or what this is, and also your toolchain. :)
Can you please take it over?

Some performance gain could be also achieved for the "safe"-version like this:

var i=0,HEX=[];
while (i<256) HEX[i]=(256+i++).toString(16).substring(1);

export default function (len) {
	len||(len=16);
	var str='',lh =((1+len)>>1)-1,num=lh;
	while (num--) str += HEX[255*Math.random()|0];
	return function () {
		if (num === 255) {
			str=''; num=lh;
			while (num--) str += HEX[255*Math.random()|0];
		}
		return str + HEX[++num];
	};
}

[lukeed]

If you just add a new file I’ll take care of the rest :)

[Uzlopak]

done

[lukeed]

I think something is off with the index tracking? I'm running the collisions script (node test/collisions.js 8 10000000) and I'm getting 11k to 15k collisions per 10M items. When I run it on the base branch, I get 9.2k to 10.3k collisions per 10M items.

[Uzlopak]

Math.random returns a number between 0 (inclusive) and 1 (exclusive). So it would only be number between 0 and 254 and not 255, increasing the collisions. Fixed it.

[Uzlopak]

The benchmarks dont work. I need to change it to const hexoid = require('../dist').hexoid; to get it work again.

[Uzlopak]

Ok.. odd length fails now. Any wish how to solve it? I assume, that was the original reason for doing the substring?

[Uzlopak]

Or was it already broken before?

[lukeed]

Right, that looks more correct, but I also realized that odd-numbered lengths are not respected. That's part of what the substring was for previously.

Updated the benchmark usage too, btw.

[lukeed]

Odd lengths worked previously. Updating the base branch w/ cherry picked commits

[Uzlopak]

I think i could unify both, normal and unsafe, and enable unsafe by setting a second parameter to true. Should increase the maintanability of the code...

[lukeed]

I prefer the multiple files – doesn't affect maintainability at all. Plus any presence of new Function is a violation in Workers, regardless of its code path execution

[Uzlopak]

An ok. Didnt know that.

[Uzlopak]

@lukeed can you have look at it,plz?

[Uzlopak]

Can we call it something different than unsafe? It does not really sound appealing to use "unsafe". I am currently thinking how we could name it.

[Uzlopak]

Can we call it jit for just-in-time?

[Uzlopak]

@lukeed
Whats up?

[lukeed]

Sorry traveling

[Uzlopak]

Yeah enjoy your travelling ;). I am currently in Paris travelling. I was just wondering if there is a technical reason for stalling this PR ;).

So again: Enjoy your travelling. :)

[Uzlopak]

I renamed unsafe to jit

[Uzlopak]

@lukeed

Are you taking a sabbatical :)?

[Uzlopak]

Closing due to inactivity. I also adapted my changes directly into fastify-multipart.