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

Improve performance of hashObject #594

Merged
merged 4 commits into from
Nov 25, 2023
Merged

Conversation

yosiat
Copy link
Contributor

@yosiat yosiat commented Oct 24, 2023

Since label names are predefined ahead-of-time and constant during
runtime, we can sort them before-hand and use them when running hashObject.

Benchmarks

Running on node v18.18.2.
Note: the registry benchmarks I couldn't run because of OOM (and on master branch as well).


http fetch GET 200 https://registry.npmjs.org/prom-client 83ms
http fetch GET 200 https://registry.npmjs.org/prom-client/-/prom-client-15.0.0.tgz 42ms
+ prom-client@15.0.0
added 4 packages from 4 contributors and audited 4 packages in 0.382s
found 0 vulnerabilities

- histogram ➭ observe#1 with 64 ➭ current x 104,381 ops/sec ±0.42% (99 runs sampled)
- histogram ➭ observe#1 with 64 ➭ prom-client@latest x 103,709 ops/sec ±0.72% (96 runs sampled)
- histogram ➭ observe#2 with 8 ➭ current x 68,178 ops/sec ±0.20% (98 runs sampled)
- histogram ➭ observe#2 with 8 ➭ prom-client@latest x 53,510 ops/sec ±0.18% (97 runs sampled)
- histogram ➭ observe#2 with 4 and 2 with 2 ➭ current x 34,169 ops/sec ±0.22% (97 runs sampled)
- histogram ➭ observe#2 with 4 and 2 with 2 ➭ prom-client@latest x 28,170 ops/sec ±0.32% (97 runs sampled)
- histogram ➭ observe#2 with 2 and 2 with 4 ➭ current x 36,057 ops/sec ±0.36% (97 runs sampled)
- histogram ➭ observe#2 with 2 and 2 with 4 ➭ prom-client@latest x 28,804 ops/sec ±0.20% (98 runs sampled)
- histogram ➭ observe#6 with 2 ➭ current x 23,360 ops/sec ±0.14% (97 runs sampled)
- histogram ➭ observe#6 with 2 ➭ prom-client@latest x 19,420 ops/sec ±0.36% (96 runs sampled)
- counter ➭ inc ➭ current x 59,643,643 ops/sec ±0.13% (101 runs sampled)
- counter ➭ inc ➭ prom-client@latest x 37,023,723 ops/sec ±0.46% (95 runs sampled)
- counter ➭ inc with labels ➭ current x 84,762 ops/sec ±0.25% (97 runs sampled)
- counter ➭ inc with labels ➭ prom-client@latest x 67,225 ops/sec ±0.62% (95 runs sampled)
- gauge ➭ inc ➭ current x 61,652,788 ops/sec ±0.41% (96 runs sampled)
- gauge ➭ inc ➭ prom-client@latest x 31,247,588 ops/sec ±0.28% (98 runs sampled)
- gauge ➭ inc with labels ➭ current x 86,552 ops/sec ±0.54% (97 runs sampled)
- gauge ➭ inc with labels ➭ prom-client@latest x 64,590 ops/sec ±0.47% (95 runs sampled)
- summary ➭ observe#1 with 64 ➭ current x 87,909 ops/sec ±0.18% (98 runs sampled)
- summary ➭ observe#1 with 64 ➭ prom-client@latest x 94,806 ops/sec ±0.87% (89 runs sampled)
- summary ➭ observe#2 with 8 ➭ current x 61,909 ops/sec ±0.15% (99 runs sampled)
- summary ➭ observe#2 with 8 ➭ prom-client@latest x 49,337 ops/sec ±0.35% (100 runs sampled)
- summary ➭ observe#2 with 4 and 2 with 2 ➭ current x 32,320 ops/sec ±0.20% (100 runs sampled)
- summary ➭ observe#2 with 4 and 2 with 2 ➭ prom-client@latest x 27,597 ops/sec ±0.79% (95 runs sampled)
- summary ➭ observe#2 with 2 and 2 with 4 ➭ current x 32,187 ops/sec ±0.21% (100 runs sampled)
- summary ➭ observe#2 with 2 and 2 with 4 ➭ prom-client@latest x 27,513 ops/sec ±0.37% (93 runs sampled)
- summary ➭ observe#6 with 2 ➭ current x 22,375 ops/sec ±0.47% (94 runs sampled)
- summary ➭ observe#6 with 2 ➭ prom-client@latest x 19,161 ops/sec ±0.14% (100 runs sampled)

┌───────────────────────────────┬────────────────────┬────────────────────┐
│ histogram                     │ current            │ prom-client@latest │
├───────────────────────────────┼────────────────────┼────────────────────┤
│ observe#1 with 64             │ 104381.0327549696  │ 103709.41386205897 │
├───────────────────────────────┼────────────────────┼────────────────────┤
│ observe#2 with 8              │ 68178.1543735591   │ 53509.94862568404  │
├───────────────────────────────┼────────────────────┼────────────────────┤
│ observe#2 with 4 and 2 with 2 │ 34169.376174278514 │ 28170.10920967872  │
├───────────────────────────────┼────────────────────┼────────────────────┤
│ observe#2 with 2 and 2 with 4 │ 36056.509189653516 │ 28804.04102513799  │
├───────────────────────────────┼────────────────────┼────────────────────┤
│ observe#6 with 2              │ 23359.579524196415 │ 19419.640968121184 │
└───────────────────────────────┴────────────────────┴────────────────────┘

┌─────────────────┬───────────────────┬────────────────────┐
│ counter         │ current           │ prom-client@latest │
├─────────────────┼───────────────────┼────────────────────┤
│ inc             │ 59643642.79575977 │ 37023723.28415886  │
├─────────────────┼───────────────────┼────────────────────┤
│ inc with labels │ 84761.87024308582 │ 67224.57758629428  │
└─────────────────┴───────────────────┴────────────────────┘

┌─────────────────┬───────────────────┬────────────────────┐
│ gauge           │ current           │ prom-client@latest │
├─────────────────┼───────────────────┼────────────────────┤
│ inc             │ 61652788.35302279 │ 31247587.722701166 │
├─────────────────┼───────────────────┼────────────────────┤
│ inc with labels │ 86551.57212390135 │ 64589.60038388497  │
└─────────────────┴───────────────────┴────────────────────┘

┌───────────────────────────────┬────────────────────┬────────────────────┐
│ summary                       │ current            │ prom-client@latest │
├───────────────────────────────┼────────────────────┼────────────────────┤
│ observe#1 with 64             │ 87908.71972414361  │ 94806.18075509806  │
├───────────────────────────────┼────────────────────┼────────────────────┤
│ observe#2 with 8              │ 61909.18982275139  │ 49336.623224028765 │
├───────────────────────────────┼────────────────────┼────────────────────┤
│ observe#2 with 4 and 2 with 2 │ 32319.8211562837   │ 27596.72566017053  │
├───────────────────────────────┼────────────────────┼────────────────────┤
│ observe#2 with 2 and 2 with 4 │ 32187.468085238048 │ 27513.002743185254 │
├───────────────────────────────┼────────────────────┼────────────────────┤
│ observe#6 with 2              │ 22375.38970678346  │ 19160.867264237862 │
└───────────────────────────────┴────────────────────┴────────────────────┘

✓ histogram ➭ observe#1 with 64 is 0.6476% faster.
✓ histogram ➭ observe#2 with 8 is 27.41% faster.
✓ histogram ➭ observe#2 with 4 and 2 with 2 is 21.30% faster.
✓ histogram ➭ observe#2 with 2 and 2 with 4 is 25.18% faster.
✓ histogram ➭ observe#6 with 2 is 20.29% faster.
✓ counter ➭ inc is 61.10% faster.
✓ counter ➭ inc with labels is 26.09% faster.
✓ gauge ➭ inc is 97.30% faster.
✓ gauge ➭ inc with labels is 34.00% faster.
⚠ summary ➭ observe#1 with 64 is 7.846% acceptably slower.
✓ summary ➭ observe#2 with 8 is 25.48% faster.
✓ summary ➭ observe#2 with 4 and 2 with 2 is 17.11% faster.
✓ summary ➭ observe#2 with 2 and 2 with 4 is 16.99% faster.
✓ summary ➭ observe#6 with 2 is 16.78% faster.

benchmarks/counter.js Outdated Show resolved Hide resolved
@zbjornson
Copy link
Collaborator

Thanks for the PR. This has been proposed before in #373 and would make it impossible to implement #298/#368. See also this comment about validly accepting subsets of labels: #373 (comment).

@yosiat
Copy link
Contributor Author

yosiat commented Oct 26, 2023

@zbjornson thanks for the feedback.

First regarding the tests example in the comments:

const prom = require('.');

async function test1() {
	instance = new prom.Gauge({
		name: 'name_2',
		help: 'help',
		labelNames: ['code', 'success'],
	});
	instance.inc({ code: '200' }, 10);
	const str = await prom.Registry.globalRegistry.getSingleMetricAsString('name_2');
	console.log(str);
}

async function test2() {
	instance = new prom.Gauge({
		name: 'test',
		help: 'help',
		labelNames: ['code', 'success', 'ok'],
	});
	instance.inc({ code: '200' }, 10);
	instance.inc({ code: '200', ok: 'yes' }, 10);
	instance.inc({ ok: 'yes', success: 'false' }, 10);
	instance.inc({ success: 'false', ok: 'no' }, 10);
	const str = await prom.Registry.globalRegistry.getSingleMetricAsString('test');
	console.log(str);
}

async function main() {
	await test1();
	console.log("\n\n");
	await test2();
}

main();

I am getting this output:

# HELP name_2 help
# TYPE name_2 gauge
name_2{code="200"} 10



# HELP test help
# TYPE test gauge
test{code="200"} 10
test{code="200",ok="yes"} 10
test{ok="yes",success="false"} 10
test{success="false",ok="no"} 10

Which is the exact same output as master branch.

Regarding support of dynamic labels, I have strong opinion against this, since it can lead to a mess in metrics on large-scale codebases and having labels predefined gives more strictness.
But, I don't think my PR is blocking having dynamic labels - see hashObject - if I don't get labelNames I fallback to extracting them like the old code.

Please re-consider this change, and I am happy to get more feedback here.

@zbjornson
Copy link
Collaborator

if I don't get labelNames I fallback to extracting them like the old code.

My bad, I missed the fallback. That's a great solution then.

Can you add a changelog entry please?

lib/util.js Outdated
}
// else

let keys = Object.keys(labels);
if (keys.length > 1) {
keys = keys.sort(); // need consistency across calls
Copy link
Collaborator

Choose a reason for hiding this comment

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

If you're making other changes, you could remove the useless keys = bit for clarity.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed the assignment.

@yosiat
Copy link
Contributor Author

yosiat commented Oct 26, 2023

@zbjornson added changelog entry, let me know if it's ok.

lib/util.js Outdated
for (let i = 0; i < keys.length; i++) {
const key = keys[i];
const value = labels[key];
if (typeof value === 'undefined') continue;
Copy link
Collaborator

@SimenB SimenB Oct 27, 2023

Choose a reason for hiding this comment

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

is typeof value === 'undefined' faster than just value === undefined? One cannot redefine undefined in a strict context afaik, so it shouldn't be needed to workaround

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it's hard in benchmarks to see the difference between the two options here, I think it falls within a margin of error.

I ran the benchmarks with typeof and === undefined and I am getting sometimes that === undefined is faster and sometimes it's slower. It's probably because I am running the benchmarks on my laptop and there is lots of variance.

I looked at difference of v8 bytecode and looks like === undefined have optimized bytecode:

typeof x === 'undefined'
         0x222300199ff0 @   40 : 20 05             TestTypeOf #5
         0x222300199ff2 @   42 : 9a 04             JumpIfFalse [4] (0x222300199ff6 @ 46)
         0x222300199ff4 @   44 : 8b 1f             Jump [31] (0x22230019a013 @ 75)

x === undefined
         0x2c0a00199ff0 @   40 : 9e 04             JumpIfNotUndefined [4] (0x2c0a00199ff4 @ 44)

And I see that lodash is doing === undefined.

So I am changing to === undefined check.

Since label names are predefined ahead-of-time and constant during
runtime, we can sort them before-hand and use them when running `hashObject.

node v18.18.2

```

http fetch GET 200 https://registry.npmjs.org/prom-client 83ms
http fetch GET 200 https://registry.npmjs.org/prom-client/-/prom-client-15.0.0.tgz 42ms
+ prom-client@15.0.0
added 4 packages from 4 contributors and audited 4 packages in 0.382s
found 0 vulnerabilities

- histogram ➭ observe#1 with 64 ➭ current x 104,381 ops/sec ±0.42% (99 runs sampled)
- histogram ➭ observe#1 with 64 ➭ prom-client@latest x 103,709 ops/sec ±0.72% (96 runs sampled)
- histogram ➭ observe#2 with 8 ➭ current x 68,178 ops/sec ±0.20% (98 runs sampled)
- histogram ➭ observe#2 with 8 ➭ prom-client@latest x 53,510 ops/sec ±0.18% (97 runs sampled)
- histogram ➭ observe#2 with 4 and 2 with 2 ➭ current x 34,169 ops/sec ±0.22% (97 runs sampled)
- histogram ➭ observe#2 with 4 and 2 with 2 ➭ prom-client@latest x 28,170 ops/sec ±0.32% (97 runs sampled)
- histogram ➭ observe#2 with 2 and 2 with 4 ➭ current x 36,057 ops/sec ±0.36% (97 runs sampled)
- histogram ➭ observe#2 with 2 and 2 with 4 ➭ prom-client@latest x 28,804 ops/sec ±0.20% (98 runs sampled)
- histogram ➭ observe#6 with 2 ➭ current x 23,360 ops/sec ±0.14% (97 runs sampled)
- histogram ➭ observe#6 with 2 ➭ prom-client@latest x 19,420 ops/sec ±0.36% (96 runs sampled)
- counter ➭ inc ➭ current x 59,643,643 ops/sec ±0.13% (101 runs sampled)
- counter ➭ inc ➭ prom-client@latest x 37,023,723 ops/sec ±0.46% (95 runs sampled)
- counter ➭ inc with labels ➭ current x 84,762 ops/sec ±0.25% (97 runs sampled)
- counter ➭ inc with labels ➭ prom-client@latest x 67,225 ops/sec ±0.62% (95 runs sampled)
- gauge ➭ inc ➭ current x 61,652,788 ops/sec ±0.41% (96 runs sampled)
- gauge ➭ inc ➭ prom-client@latest x 31,247,588 ops/sec ±0.28% (98 runs sampled)
- gauge ➭ inc with labels ➭ current x 86,552 ops/sec ±0.54% (97 runs sampled)
- gauge ➭ inc with labels ➭ prom-client@latest x 64,590 ops/sec ±0.47% (95 runs sampled)
- summary ➭ observe#1 with 64 ➭ current x 87,909 ops/sec ±0.18% (98 runs sampled)
- summary ➭ observe#1 with 64 ➭ prom-client@latest x 94,806 ops/sec ±0.87% (89 runs sampled)
- summary ➭ observe#2 with 8 ➭ current x 61,909 ops/sec ±0.15% (99 runs sampled)
- summary ➭ observe#2 with 8 ➭ prom-client@latest x 49,337 ops/sec ±0.35% (100 runs sampled)
- summary ➭ observe#2 with 4 and 2 with 2 ➭ current x 32,320 ops/sec ±0.20% (100 runs sampled)
- summary ➭ observe#2 with 4 and 2 with 2 ➭ prom-client@latest x 27,597 ops/sec ±0.79% (95 runs sampled)
- summary ➭ observe#2 with 2 and 2 with 4 ➭ current x 32,187 ops/sec ±0.21% (100 runs sampled)
- summary ➭ observe#2 with 2 and 2 with 4 ➭ prom-client@latest x 27,513 ops/sec ±0.37% (93 runs sampled)
- summary ➭ observe#6 with 2 ➭ current x 22,375 ops/sec ±0.47% (94 runs sampled)
- summary ➭ observe#6 with 2 ➭ prom-client@latest x 19,161 ops/sec ±0.14% (100 runs sampled)

┌───────────────────────────────┬────────────────────┬────────────────────┐
│ histogram                     │ current            │ prom-client@latest │
├───────────────────────────────┼────────────────────┼────────────────────┤
│ observe#1 with 64             │ 104381.0327549696  │ 103709.41386205897 │
├───────────────────────────────┼────────────────────┼────────────────────┤
│ observe#2 with 8              │ 68178.1543735591   │ 53509.94862568404  │
├───────────────────────────────┼────────────────────┼────────────────────┤
│ observe#2 with 4 and 2 with 2 │ 34169.376174278514 │ 28170.10920967872  │
├───────────────────────────────┼────────────────────┼────────────────────┤
│ observe#2 with 2 and 2 with 4 │ 36056.509189653516 │ 28804.04102513799  │
├───────────────────────────────┼────────────────────┼────────────────────┤
│ observe#6 with 2              │ 23359.579524196415 │ 19419.640968121184 │
└───────────────────────────────┴────────────────────┴────────────────────┘

┌─────────────────┬───────────────────┬────────────────────┐
│ counter         │ current           │ prom-client@latest │
├─────────────────┼───────────────────┼────────────────────┤
│ inc             │ 59643642.79575977 │ 37023723.28415886  │
├─────────────────┼───────────────────┼────────────────────┤
│ inc with labels │ 84761.87024308582 │ 67224.57758629428  │
└─────────────────┴───────────────────┴────────────────────┘

┌─────────────────┬───────────────────┬────────────────────┐
│ gauge           │ current           │ prom-client@latest │
├─────────────────┼───────────────────┼────────────────────┤
│ inc             │ 61652788.35302279 │ 31247587.722701166 │
├─────────────────┼───────────────────┼────────────────────┤
│ inc with labels │ 86551.57212390135 │ 64589.60038388497  │
└─────────────────┴───────────────────┴────────────────────┘

┌───────────────────────────────┬────────────────────┬────────────────────┐
│ summary                       │ current            │ prom-client@latest │
├───────────────────────────────┼────────────────────┼────────────────────┤
│ observe#1 with 64             │ 87908.71972414361  │ 94806.18075509806  │
├───────────────────────────────┼────────────────────┼────────────────────┤
│ observe#2 with 8              │ 61909.18982275139  │ 49336.623224028765 │
├───────────────────────────────┼────────────────────┼────────────────────┤
│ observe#2 with 4 and 2 with 2 │ 32319.8211562837   │ 27596.72566017053  │
├───────────────────────────────┼────────────────────┼────────────────────┤
│ observe#2 with 2 and 2 with 4 │ 32187.468085238048 │ 27513.002743185254 │
├───────────────────────────────┼────────────────────┼────────────────────┤
│ observe#6 with 2              │ 22375.38970678346  │ 19160.867264237862 │
└───────────────────────────────┴────────────────────┴────────────────────┘

✓ histogram ➭ observe#1 with 64 is 0.6476% faster.
✓ histogram ➭ observe#2 with 8 is 27.41% faster.
✓ histogram ➭ observe#2 with 4 and 2 with 2 is 21.30% faster.
✓ histogram ➭ observe#2 with 2 and 2 with 4 is 25.18% faster.
✓ histogram ➭ observe#6 with 2 is 20.29% faster.
✓ counter ➭ inc is 61.10% faster.
✓ counter ➭ inc with labels is 26.09% faster.
✓ gauge ➭ inc is 97.30% faster.
✓ gauge ➭ inc with labels is 34.00% faster.
⚠ summary ➭ observe#1 with 64 is 7.846% acceptably slower.
✓ summary ➭ observe#2 with 8 is 25.48% faster.
✓ summary ➭ observe#2 with 4 and 2 with 2 is 17.11% faster.
✓ summary ➭ observe#2 with 2 and 2 with 4 is 16.99% faster.
✓ summary ➭ observe#6 with 2 is 16.78% faster.
```
@yosiat
Copy link
Contributor Author

yosiat commented Nov 10, 2023

@zbjornson @SimenB after addressing your questions & concerns, can we move forward with this PR? or there is something left here?

Copy link
Collaborator

@zbjornson zbjornson left a comment

Choose a reason for hiding this comment

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

Looks great, thanks for the contribution. Sorry again for the off-base review earlier.

@zbjornson zbjornson merged commit efdb283 into siimon:master Nov 25, 2023
10 checks passed
@yosiat yosiat deleted the perf-hashobject branch November 25, 2023 09:24
@SimenB
Copy link
Collaborator

SimenB commented Dec 15, 2023

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

3 participants