[Refactor] improve performance of rule merging

[golopot]

The performance for merged rule in Component.detect is improved. In an end-to-end linting benchmark this change is 4% faster.

Benchmark (mean of 5 runs):

- 5.164 s
+ 4.966 s

[codecov]

Codecov Report

Merging #3281 (4bc7669) into master (c8833f3) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

❗ Current head 4bc7669 differs from pull request most recent head 438c21a. Consider uploading reports for the commit 438c21a to get more accurate results

@@            Coverage Diff             @@
##           master    #3281      +/-   ##
==========================================
- Coverage   97.72%   97.72%   -0.01%     
==========================================
  Files         123      123              
  Lines        8745     8744       -1     
  Branches     3170     3165       -5     
==========================================
- Hits         8546     8545       -1     
  Misses        199      199              

Impacted Files	Coverage Δ
lib/util/Components.js	98.72% <100.00%> (-0.01%)	⬇️

---

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 c8833f3...438c21a. Read the comment docs.

[golopot]

This property access performed for each AST node turned out to be performance bottleneck.

[ljharb]

for-of is slow; fns.forEach((node) => fn(node)) is almost certainly faster (and cleaner; loops are bad)

[golopot]

for-of is not slow in node, v8 have done optimizations to make it performs comparable to for loop.

const N = 100000;
const length = 100;

const array = new Array(length).fill().map((_, i) => i);

function benchmarkForLoop() {
  let start = performance.now();
  let x = 0;
  for (let i = 0; i < N; i++) {
    for (let j = 0; j < array.length; j++) {
      x = (x + array[j]) % 7;
    }
  }
  let end = performance.now();
  console.log(end - start);
}

function benchmarkForOfLoop() {
  let start = performance.now();
  let x = 0;
  for (let i = 0; i < N; i++) {
    for (const y of array) {
      x = (x + y) % 7;
    }
  }
  let end = performance.now();
  console.log(end - start);
}

function benchmarkForEach() {
  let start = performance.now();
  let x = 0;
  for (let i = 0; i < N; i++) {
    array.forEach((y) => {
      x = (x + y) % 7;
    });
  }
  let end = performance.now();
  console.log(end - start);
}

benchmarkForLoop();
benchmarkForOfLoop();
benchmarkForEach();

// Output:
// 42.265081003308296
// 47.21948900818825
// 57.372923001646996

[ljharb]

Try those benchmarks without using Array(), which creates a sparse array that .fill may not be able to un-taint.

[golopot]

Done!

[ljharb]

same here; let's use handlersByKey.forEach here

[ljharb]

Suggested change

	for (const rule of rules) {
	for (const key of Object.keys(rule)) {
	const fns = handlersByKey.get(key);
	if (!fns) {
	handlersByKey.set(key, [rule[key]]);
	} else {
	fns.push(rule[key]);
	}
	}
	}
	rules.forEach((rule) => {
	Object.keys(rule).forEach((key) => {
	const fns = handlersByKey.get(key);
	if (!fns) {
	handlersByKey.set(key, [rule[key]]);
	} else {
	fns.push(rule[key]);
	}
	});
	});

[ljharb]

is there no way to have this rule pass with this function's signature?

[golopot]

That rule has been deprecated for a long time and it lacks support for lots of typescript syntaxes.

[ljharb]

gotcha; so we'd have to use a similar rule from the typescript plugin?

[golopot]

You are looking for eslint-plugin-jsdoc .

[golopot]

typescript plugin does not handle jsdoc as far as I know