perf(format): cache breaklines and spaces as much as possible

[H4ad]

This PR basically tries to cache as much as possible the strings and spaces created since they are basically the same, there's no need to allocate new memory for those strings.

This optimization affects eol:

• \r
• \n
• \r\n

This optimization will only cache strings up to 200 characters (it was chosen just because it speedups tabSize=4 for this huge json).

In the happy path, this algorithm will only allocate memory for the array and the objects, but not for the content.

For smaller files, this change didn't make the code run faster, this is only valid for larger json.

Before:

End: 2431.62ms
RSS: 2290.38671875MB
Amount of edits: 7876047

After:

End: 989.64ms
RSS: 709.5234375MB
Amount of edits: 7876047

Benchmark:

const heavyJson = require('fs').readFileSync('./rrdom-benchmark-1.json', 'utf8');

const lib = require('./lib/umd/main');

const startTime = performance.now();
const edits = lib.format(heavyJson, undefined, {
  tabSize: 2,
  insertFinalNewline: true,
  insertSpaces: true,
});
console.log(`End: ${(performance.now() - startTime).toFixed(2)}ms`);
console.log(`RSS: ${process.memoryUsage.rss() / 1024 / 1024}MB`);
console.log(`Amount of edits: ${edits.length}`);

Large Json: rrdom-benchmark-1.json.zip

[H4ad]

@microsoft-github-policy-service agree

[ljharb]

the ?? operator is a breaking change for node 12. since there's no engines declaration, everything that worked in 3.0.0 is part of the public API.

[H4ad]

The minimum version of this package is 14.x, so I think this is expected, it worked before because of luck not because was the intention.

An PR can be created to fix this but probably this can happen more times in the future.

[ljharb]

That's only the case if it's declared in engines.node - intention doesn't matter for semver.

The way to avoid it happening in the future, after fixing it and releasing a patch, is to either add node 12 into the CI matrix, or, to immediately add engines.node and do a major bump.

[aeschli]

Yes, the minimum is now 16.x.

[ljharb]

Then this should have been a v4.0.0 that included an explicit engines.node declaration.