Skip to content

Conversation

jakebailey
Copy link
Member

I was profiling the benchmarks from #5; one thing that stuck out to me was that the most allocations happened here:

image

This is weird, why would that line allocate? Then I remembered the cost of bound method wrappers... By binding b.bind with a receiver just once, we avoid quite literally 86% of all allocations during bind, save 14% of the memory, and save 17% of the time.

goos: linux
goarch: amd64
pkg: github.com/microsoft/typescript-go/internal/compiler
cpu: Intel(R) Core(TM) i9-10900K CPU @ 3.70GHz
         │   old.txt   │               new.txt               │
         │   sec/op    │   sec/op     vs base                │
Bind-20    24.74m ± 2%   20.55m ± 1%  -16.94% (p=0.000 n=10)
Parse-20   46.17m ± 1%   46.07m ± 2%        ~ (p=0.684 n=10)
geomean    33.80m        30.77m        -8.96%

         │   old.txt    │               new.txt                │
         │     B/op     │     B/op      vs base                │
Bind-20    9.776Mi ± 0%   8.420Mi ± 0%  -13.87% (p=0.000 n=10)
Parse-20   23.59Mi ± 0%   23.59Mi ± 0%        ~ (p=0.579 n=10)
geomean    15.19Mi        14.09Mi        -7.19%

         │   old.txt    │               new.txt               │
         │  allocs/op   │  allocs/op   vs base                │
Bind-20    102.56k ± 0%   13.69k ± 0%  -86.65% (p=0.000 n=10)
Parse-20    239.1k ± 0%   239.1k ± 0%        ~ (p=0.537 n=10)
geomean     156.6k        57.22k       -63.46%

This PR is in no way final (I just made the "zero value" usable, not that it is already without bindSourceFile), but it really shows the cost of binding methods like this.

@jakebailey jakebailey requested a review from ahejlsberg October 22, 2024 04:40
@jakebailey
Copy link
Member Author

Confusingly this doesn't show up at the cli...

image

@ahejlsberg
Copy link
Member

Crazy. We obviously have to be careful about closures that don't look like closures!

@jakebailey
Copy link
Member Author

I need to do further analysis here; the big difference I see wasn't reflected in the cli, but it's possible that this only negatively affected the checker test.

@jakebailey
Copy link
Member Author

This was done in #50; also, I'm pretty sure that https://go-review.googlesource.com/c/go/+/609095 is likely to have also fixed / helped this.

@jakebailey jakebailey closed this Nov 4, 2024
@jakebailey jakebailey deleted the jabaile/prebind branch November 4, 2024 17:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants