js-libp2p-gossipsub
js-libp2p-gossipsub copied to clipboard
Performance of gauge.inc()
As found in the below profile, the total gauge.inc() calls takes > 2% of cpu time
- maybe switch to counter.inc() instead?
- write performance test to make sure
the bottom up view shows that gauge.inc() takes 2.45% of cpu time which is too much
for a comparison, runHeartBeat takes 3.6% (before the improvements)
@tuyennhv Please review also all the metrics taken for each gossip packet, there are a lot!. We should at the same time:
- Keep only necessary metrics: easy
- Check if performance can be improved for Gauge.inc(): harder
Check if performance can be improved for Gauge.inc(): harder
@dapplion gauge.inc() is slower than counter.inc() because it hash labels object 2 times, see https://github.com/siimon/prom-client/pull/503
since the hashObject() in prom-client is not cheap, if we could cache metric value and flush every n times or per scraping metric
class LabelCachedGauge<Labels extends string> {
private twoLabelsCache: Map<string, Map<string, number>> | undefined;
private oneLabelCache: Map<string, number> | undefined;
constructor(
private readonly gauge: Gauge<Labels>,
private readonly label1Key: Labels,
private readonly label2Key?: Labels
) {
if (label2Key !== undefined) {
this.twoLabelsCache = new Map();
} else {
this.oneLabelCache = new Map();
}
}
inc(label1: string, label2?: string, value = 1): void {
if (label2 !== undefined) {
let label2Cache = this.twoLabelsCache?.get(label1);
if (!label2Cache) {
label2Cache = new Map<string, number>();
this.twoLabelsCache?.set(label1, label2Cache);
}
label2Cache.set(label2, (label2Cache.get(label2) ?? 0) + value);
} else {
this.oneLabelCache?.set(label1, (this.oneLabelCache.get(label1) ?? 0) + value);
}
}
set(label1: string, label2?: string, value = 1): void {
if (label2 !== undefined) {
let label2Cache = this.twoLabelsCache?.get(label1);
if (!label2Cache) {
label2Cache = new Map<string, number>();
this.twoLabelsCache?.set(label1, label2Cache);
}
label2Cache.set(label2, value);
} else {
this.oneLabelCache?.set(label1, value);
}
}
// TODO: more methods
flush(): void {
// two label
if (this.twoLabelsCache && this.label2Key !== undefined) {
for (const [label1Value, label2Cache] of this.twoLabelsCache) {
for (const [label2Value, count] of label2Cache) {
this.gauge.inc({[this.label1Key]: label1Value, [this.label2Key]: label2Value} as LabelValues<Labels>, count);
}
}
this.twoLabelsCache.clear();
}
// one label
if (this.oneLabelCache) {
for (const [label, count] of this.oneLabelCache) {
this.gauge.inc({[this.label1Key]: label} as LabelValues<Labels>, count);
}
this.oneLabelCache.clear();
}
}
}
class NoLabelCachedGauge {
private cache = 0
private gauge: Gauge<string>
constructor(gauge: Gauge<string>) {
this.gauge = gauge;
}
inc(value = 1): void {
this.cache += value;
}
// TODO: add more methods
flush(): void {
this.gauge.inc(this.cache);
this.cache = 0;
}
}
the performance shows that if we we call inc() and flush() per 100 times:
- for no label gauge, it's 7x faster
- for one label gauge, it's 1.2x faster
- for 2 label gauge, it's 2.5x faster
This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.