Skip to content

Conversation

dragomirtitian
Copy link
Contributor

Fixes #56960

This PR implements the suggested improvement of not converting between Buffer and string unless absolutely necessary in the virtual file system implementation used for tests.

Some anecdotal tests:

Environment main PR Win%
Linux, 6 workers node 18 01:59 01:49 8.40%
Windows 10, 11 workers node 20 03:05 02:55 5.41%

This also helps with isolated declarations clawing back some of the time taken by ID tests:

Environment main PR Win%
Linux, 6 workers node 18 02:12 01:58 10.61%
Windows 10, 11 workers node 20 03:20 03:10 5.00%

@typescript-bot typescript-bot added the For Uncommitted Bug PR for untriaged, rejected, closed or missing bug label Jan 5, 2024
@dragomirtitian dragomirtitian force-pushed the buffer-to-string-conversions branch from 2b74554 to b41fc37 Compare January 5, 2024 16:00
Copy link
Member

@jakebailey jakebailey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems reasonable to me, though I am not seeing quite the same speedup on my machine.

I'm interested in @sheetalkamat's thoughts.

@typescript-bot typescript-bot added For Backlog Bug PRs that fix a backlog bug and removed For Uncommitted Bug PR for untriaged, rejected, closed or missing bug labels Jan 5, 2024
@dragomirtitian
Copy link
Contributor Author

This seems reasonable to me, though I am not seeing quite the same speedup on my machine.

I am curios if you are seeing any speedups on your machine?

@jakebailey
Copy link
Member

Barely, if anything; I can try and get more data but I ran it a couple times and saw roughly my normal 1m45s before and after.

@jakebailey
Copy link
Member

$ hyperfine --prepare 'git switch main' -n 'main' 'npm test -- --no-lint' --prepare 'git switch pr-56961' -n 'PR 56961' 'npm test -- --no-lint'
Benchmark 1: main
  Time (mean ± σ):     114.415 s ±  1.469 s    [User: 2134.875 s, System: 51.155 s]
  Range (min … max):   110.731 s … 116.508 s    10 runs
 
Benchmark 2: PR 56961
  Time (mean ± σ):     109.475 s ±  1.874 s    [User: 2032.384 s, System: 37.635 s]
  Range (min … max):   108.442 s … 114.625 s    10 runs
 
Summary
  'PR 56961' ran
    1.05 ± 0.02 times faster than 'main'

@jakebailey
Copy link
Member

Perhaps this has a better effect when fewer workers are in use; my machine ends up running with 19 workers.

@jakebailey jakebailey merged commit ea9aed8 into microsoft:main Jan 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
For Backlog Bug PRs that fix a backlog bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Reduce Buffer to string round trip conversions in tests
3 participants