Skip to content

Port TS PR 61505 - Cache mapper instantiations #1358

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

jakebailey
Copy link
Member

@jakebailey jakebailey commented Jul 3, 2025

This is an early port of microsoft/TypeScript#61505, because I like to go fast

Calculating the key increases allocs by a bit (really wish we had the yet-to-exist Go custom map implementation), but for https://github.com/AnswerOverflow/AnswerOverflow/tree/1ab687793a598789a0c86b4ca5f17ee6f3a256aa/apps/dashboard:

TypeScript 5.8 vs 5.9:

Files:              3351
Lines:            573983
Identifiers:      646173
Symbols:          652920
Types:            139674
Instantiations: 13672403
Memory used:     912429K
I/O read:          0.13s
I/O write:         0.00s
Parse time:        1.91s
Bind time:         0.60s
Check time:        7.67s
Emit time:         0.12s
Total time:       10.30s
Files:             3353
Lines:           575420
Identifiers:     646578
Symbols:         651905
Types:           139189
Instantiations:  797184
Memory used:    898938K
I/O read:         0.10s
I/O write:        0.00s
Parse time:       1.84s
Bind time:        0.61s
Check time:       3.79s
Emit time:        0.11s
Total time:       6.35s

tsgo main vs this PR;

Files:              3501
Lines:            601735
Identifiers:      661699
Symbols:         1219580
Types:            420852
Instantiations: 25441996
Memory used:     862438K
Memory allocs:  63315601
Config time:      0.002s
Parse time:       0.170s
Bind time:        0.044s
Check time:       2.034s
Emit time:        0.000s
Total time:       2.257s
Files:              3501
Lines:            601735
Identifiers:      661699
Symbols:         1219580
Types:            419356
Instantiations:  2178836
Memory used:     861708K
Memory allocs:  12761985
Config time:      0.003s
Parse time:       0.163s
Bind time:        0.048s
Check time:       1.107s
Emit time:        0.000s
Total time:       1.327s

(Instantiations look higher than 5.8 or 5.9 because tsgo's metrics sum up instantiations between all concurrent checkers)

Benchmark 1: ~/work/TypeScript-go/built/local-old/tsgo
  Time (mean ± σ):      2.270 s ±  0.078 s    [User: 13.092 s, System: 1.112 s]
  Range (min … max):    2.164 s …  2.392 s    10 runs

Benchmark 2: ~/work/TypeScript-go/built/local/tsgo
  Time (mean ± σ):      1.376 s ±  0.069 s    [User: 7.626 s, System: 1.028 s]
  Range (min … max):    1.293 s …  1.531 s    10 runs

Summary
  ~/work/TypeScript-go/built/local/tsgo ran
    1.65 ± 0.10 times faster than ~/work/TypeScript-go/built/local-old/tsgo
Benchmark 1 (3 runs): /home/jabaile/work/TypeScript-go/built/local-old/tsgo
  measurement          mean ± σ            min … max           outliers         delta
  wall_time          2.29s  ± 90.2ms    2.23s  … 2.39s           0 ( 0%)        0%
  peak_rss           1.46GB ± 92.0MB    1.36GB … 1.55GB          0 ( 0%)        0%
  cpu_cycles         59.7G  ± 1.84G     57.7G  … 61.1G           0 ( 0%)        0%
  instructions       85.3G  ± 2.30G     82.7G  … 87.2G           0 ( 0%)        0%
  cache_references   1.19G  ± 21.0M     1.17G  … 1.22G           0 ( 0%)        0%
  cache_misses        251M  ± 6.31M      244M  …  256M           0 ( 0%)        0%
  branch_misses       177M  ± 1.79M      175M  …  179M           0 ( 0%)        0%
Benchmark 2 (4 runs): /home/jabaile/work/TypeScript-go/built/local/tsgo
  measurement          mean ± σ            min … max           outliers         delta
  wall_time          1.37s  ± 80.5ms    1.30s  … 1.47s           0 ( 0%)        ⚡- 40.1% ±  7.3%
  peak_rss           1.13GB ± 7.47MB    1.12GB … 1.14GB          0 ( 0%)        ⚡- 22.8% ±  7.9%
  cpu_cycles         34.1G  ± 1.02G     33.3G  … 35.5G           0 ( 0%)        ⚡- 42.9% ±  4.6%
  instructions       40.8G  ±  912M     39.9G  … 41.7G           0 ( 0%)        ⚡- 52.2% ±  3.7%
  cache_references    979M  ± 17.1M      963M  … 1.00G           0 ( 0%)        ⚡- 18.0% ±  3.1%
  cache_misses        154M  ± 8.52M      146M  …  165M           0 ( 0%)        ⚡- 38.5% ±  6.0%
  branch_misses       109M  ± 2.03M      107M  …  112M           0 ( 0%)        ⚡- 38.8% ±  2.1%

@Copilot Copilot AI review requested due to automatic review settings July 3, 2025 23:37
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR ports TypeScript’s PR 61505 to introduce caching for type mapper instantiations, aiming to reduce repeated instantiation overhead and improve performance.

  • Added a stack of active mappers with per-mapper caches backed by sync.Pool.
  • Updated instantiateTypeWithAlias to check and store in those caches.
  • Cleared mapper caches after each inference in getInferredType.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
internal/checker/inference.go Inserted clearActiveMapperCaches() after each inference loop
internal/checker/checker.go Added activeMappers, activeTypeMappersCaches, cache logic in instantiateTypeWithAlias, and push/pop/clear helper methods
Comments suppressed due to low confidence (2)

internal/checker/inference.go:1332

  • [nitpick] clearActiveMapperCaches() is invoked inside the loop for each inference, leading to repeated map clears. Consider moving this call outside the loop or only once per context to avoid redundant operations.
		c.clearActiveMapperCaches()

@Andarist
Copy link
Contributor

Andarist commented Jul 4, 2025

Okay. I approve.

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