js-libp2p-gossipsub icon indicating copy to clipboard operation
js-libp2p-gossipsub copied to clipboard

Performance of gauge.inc()

Open twoeths opened this issue 3 years ago • 5 comments
trafficstars

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
Screen Shot 2022-06-10 at 17 07 03

0522_contabo_20_subscribe_all_subnets.cpuprofile.zip

twoeths avatar Jun 10 '22 10:06 twoeths

the bottom up view shows that gauge.inc() takes 2.45% of cpu time which is too much

Screen Shot 2022-06-15 at 10 15 25

for a comparison, runHeartBeat takes 3.6% (before the improvements)

Screen Shot 2022-06-15 at 10 17 02

twoeths avatar Jun 15 '22 03:06 twoeths

@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

dapplion avatar Jun 15 '22 07:06 dapplion

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

twoeths avatar Jun 15 '22 07:06 twoeths

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

twoeths avatar Jun 16 '22 08:06 twoeths

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.

stale[bot] avatar Aug 13 '22 14:08 stale[bot]