Skip to content

perf(pnp): store inlined pnp data in multiline string literal#3977

Merged
merceyz merged 1 commit intomasterfrom
paul/perf/pnp-inlined-string-literal
Jan 18, 2022
Merged

perf(pnp): store inlined pnp data in multiline string literal#3977
merceyz merged 1 commit intomasterfrom
paul/perf/pnp-inlined-string-literal

Conversation

@paul-soporan
Copy link
Member

@paul-soporan paul-soporan commented Jan 13, 2022 •
edited
Loading

What's the problem this PR addresses?

As @merceyz said in #3965 (comment), parsing JSON strings is more efficient than parsing JS object literals.

Closes #3965.

How did you fix it?

Made the inlined PnP data get stored in a multiline string literal (where newlines are escaped using \) containing JSON.

Why a multiline string literal over a template literal? @merceyz originally looked into using a template literal in #3965 (comment), but it turned out to be slower than even an object literal. More context: nodejs/node#41448

@merceyz ran some benchmarks on the next.js repo:

hyperfine -w 5\
  "node -r ./.pnp-object.cjs -e 1"\
  "node -r ./.pnp-string-lf.cjs -e 1"\
  "node -r ./.pnp-data-json.cjs -e 1"\
  "node -r ./.pnp-template.cjs -e 1"
Benchmark 1: node -r ./.pnp-object.cjs -e 1
  Time (mean ± σ):     155.7 ms ±   2.8 ms    [User: 163.1 ms, System: 21.4 ms]
  Range (min … max):   152.3 ms … 161.3 ms    19 runs

Benchmark 2: node -r ./.pnp-string-lf.cjs -e 1
  Time (mean ± σ):     135.1 ms ±   1.8 ms    [User: 135.6 ms, System: 32.0 ms]
  Range (min … max):   132.3 ms … 140.7 ms    21 runs

Benchmark 3: node -r ./.pnp-data-json.cjs -e 1
  Time (mean ± σ):      90.7 ms ±   0.8 ms    [User: 105.0 ms, System: 18.0 ms]
  Range (min … max):    89.3 ms …  92.6 ms    32 runs

Benchmark 4: node -r ./.pnp-template.cjs -e 1
  Time (mean ± σ):     158.0 ms ±   1.2 ms    [User: 160.7 ms, System: 29.2 ms]
  Range (min … max):   154.9 ms … 160.2 ms    18 runs

Summary
  'node -r ./.pnp-data-json.cjs -e 1' ran
    1.49 ± 0.02 times faster than 'node -r ./.pnp-string-lf.cjs -e 1'
    1.72 ± 0.03 times faster than 'node -r ./.pnp-object.cjs -e 1'
    1.74 ± 0.02 times faster than 'node -r ./.pnp-template.cjs -e 1'

Checklist

  • I have set the packages that need to be released for my changes to be effective.
  • I will check that all automated PR checks pass before the PR gets reviewed.

@merceyz merceyz changed the title perf: store inlined pnp data in multiline string literal perf(pnp): store inlined pnp data in multiline string literal Jan 13, 2022
@merceyz merceyz mentioned this pull request Jan 13, 2022
Closed
merceyz
merceyz previously approved these changes Jan 15, 2022
View reviewed changes
@merceyz merceyz force-pushed the paul/perf/pnp-inlined-string-literal branch from a20dd85 to 4e82d41 Compare January 18, 2022 11:55
@merceyz merceyz enabled auto-merge (squash) January 18, 2022 11:55
@merceyz merceyz merged commit ecfeca0 into master Jan 18, 2022
@merceyz merceyz deleted the paul/perf/pnp-inlined-string-literal branch January 18, 2022 14:01
arcanis pushed a commit that referenced this pull request Jan 18, 2022
@paul-soporan @arcanis
perf(pnp): store inlined pnp data in multiline string literal (#3977)
f81b99e
@merceyz merceyz mentioned this pull request Apr 3, 2022
Merged
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@merceyz merceyz merceyz approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@paul-soporan @merceyz