Skip to content

Commit 72f191f

Browse files
committed
Avoid repeated member lookup during full property enumeration
Full enumeration was repeating name lookups and base traversal that lazy resolution needs but enumeration has already done. This overhead appeared in the MUI and Webpack regressions after unified resolution. Preserve its selective-lookup symbol and heap savings without paying the same lookup costs while collecting all properties. Six fresh-process interleaved production CLI comparisons against 5f35280, with normal GC and default threading, reduce instructions 2.55% on CI-pinned MUI and 2.17% on Xstate. Webpack and VS Code instruction differences are inconclusive. Allocations decrease on all four workloads. Reported heap is unchanged except for a 0.12% increase on Xstate. Against main aa81492, the combined branch uses 9.74-15.75% fewer instructions and 23.11-31.55% less reported heap across these workloads. These comparisons include all earlier optimizations on the branch. MUI uses source and dependencies from the benchmark's CI-pinned commit 6780195595c12252a130aa5ae5c5bd461229c6b7. Webpack is isolated at 2289141381f2c1c965cb65118fa0ffe2e9e19834 with scenario dependencies restored. VS Code uses 8bf178e1e1c2be3723f14683f7b1e5fdb3f780e4. Xstate uses cc087bf52cbe5cc291146816e7ef530bc53698a0 and its existing partial dependency installation. Existing workload diagnostics match exactly across main, before, and after; candidate semantic counts match before, including symbol counts. MUI check time improves 10.85% in these samples, but the shared host is noisy. No check-time difference against main is significant. This is not evidence that the hosted CI timing regression has been eliminated. heap-B/op is CLI-reported heap, not a forced-GC retained-heap measure. Cached and uncached declared/inherited enumeration, identity, ordering, negative lookups, unaffected snapshots, renamed keys, and diamond bases pass focused tests with race/checkptr. npx hereby validate --all passes, including 166161 Go tests and 954 API tests. Before and after this change: │ before.txt │ after.txt │ │ instructions/op │ instructions/op vs base │ mui-pinned-20 293.6G ± 2% 286.1G ± 2% -2.55% (p=0.002 n=6) webpack-20 53.35G ± 2% 52.12G ± 3% ~ (p=0.065 n=6) xstate-20 20.56G ± 2% 20.11G ± 1% -2.17% (p=0.015 n=6) vscode-20 300.9G ± 5% 301.5G ± 3% ~ (p=1.000 n=6) geomean 99.21G 97.52G -1.71% │ before.txt │ after.txt │ │ heap-B/op │ heap-B/op vs base │ mui-pinned-20 2.061Gi ± 0% 2.061Gi ± 0% ~ (p=0.394 n=6) webpack-20 666.6Mi ± 0% 666.7Mi ± 0% ~ (p=0.240 n=6) xstate-20 274.9Mi ± 0% 275.3Mi ± 0% +0.12% (p=0.002 n=6) vscode-20 2.834Gi ± 0% 2.833Gi ± 0% ~ (p=0.485 n=6) geomean 1.005Gi 1.005Gi +0.02% │ before.txt │ after.txt │ │ allocs/op │ allocs/op vs base │ mui-pinned-20 29.96M ± 0% 29.86M ± 0% -0.34% (p=0.026 n=6) webpack-20 6.287M ± 0% 6.238M ± 0% -0.78% (p=0.002 n=6) xstate-20 3.394M ± 0% 3.371M ± 0% -0.68% (p=0.002 n=6) vscode-20 36.92M ± 0% 36.67M ± 0% -0.68% (p=0.002 n=6) geomean 12.39M 12.32M -0.62% │ before.txt │ after.txt │ │ check-sec/op │ check-sec/op vs base │ mui-pinned-20 10.155 ± 27% 9.053 ± 12% -10.85% (p=0.041 n=6) webpack-20 1.844 ± 9% 1.743 ± 12% ~ (p=0.258 n=6) xstate-20 915.5m ± 9% 926.0m ± 9% ~ (p=1.000 n=6) vscode-20 7.689 ± 5% 7.549 ± 14% ~ (p=0.818 n=6) geomean 3.388 3.240 -4.36% │ main.txt │ after.txt │ │ instructions/op │ instructions/op vs base │ mui-pinned-20 317.0G ± 6% 286.1G ± 2% -9.74% (p=0.002 n=6) webpack-20 58.36G ± 9% 52.12G ± 3% -10.68% (p=0.002 n=6) xstate-20 23.45G ± 8% 20.11G ± 1% -14.23% (p=0.002 n=6) vscode-20 357.8G ± 10% 301.5G ± 3% -15.75% (p=0.002 n=6) geomean 111.6G 97.52G -12.63% │ main.txt │ after.txt │ │ heap-B/op │ heap-B/op vs base │ mui-pinned-20 2.680Gi ± 0% 2.061Gi ± 0% -23.11% (p=0.002 n=6) webpack-20 915.2Mi ± 0% 666.7Mi ± 0% -27.15% (p=0.002 n=6) xstate-20 393.7Mi ± 0% 275.3Mi ± 0% -30.09% (p=0.002 n=6) vscode-20 4.138Gi ± 0% 2.833Gi ± 0% -31.55% (p=0.002 n=6) geomean 1.397Gi 1.005Gi -28.05% │ main.txt │ after.txt │ │ allocs/op │ allocs/op vs base │ mui-pinned-20 32.84M ± 0% 29.86M ± 0% -9.07% (p=0.002 n=6) webpack-20 6.586M ± 0% 6.238M ± 0% -5.29% (p=0.002 n=6) xstate-20 3.804M ± 0% 3.371M ± 0% -11.39% (p=0.002 n=6) vscode-20 36.87M ± 0% 36.67M ± 0% -0.57% (p=0.002 n=6) geomean 13.20M 12.32M -6.67% │ main.txt │ after.txt │ │ check-sec/op │ check-sec/op vs base │ mui-pinned-20 9.032 ± 13% 9.053 ± 12% ~ (p=1.000 n=6) webpack-20 1.740 ± 8% 1.743 ± 12% ~ (p=1.000 n=6) xstate-20 985.5m ± 18% 926.0m ± 9% ~ (p=0.180 n=6) vscode-20 8.076 ± 10% 7.549 ± 14% ~ (p=0.240 n=6) geomean 3.344 3.240 -3.09%
1 parent 5f35280 commit 72f191f

2 files changed

Lines changed: 116 additions & 7 deletions

File tree

‎tsc/internal/checker/checker.go‎

Lines changed: 58 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -19691,9 +19691,7 @@ func (c *Checker) resolveObjectTypeMember(resolution *objectMemberResolution, na
1969119691
if resolution.mapper == nil || c.isNamedMember(symbol, name) {
1969219692
result = symbol
1969319693
if instantiate && resolution.mapper != nil {
19694-
if _, unaffected := slices.BinarySearch(resolution.unaffected, name); !unaffected {
19695-
result = c.newInstantiatedSymbol(symbol, resolution.mapper)
19696-
}
19694+
result = c.instantiateObjectTypeMember(resolution, name, symbol)
1969719695
}
1969819696
}
1969919697
}
@@ -19732,6 +19730,15 @@ func (c *Checker) resolveObjectTypeMember(resolution *objectMemberResolution, na
1973219730
return result
1973319731
}
1973419732

19733+
func (c *Checker) instantiateObjectTypeMember(resolution *objectMemberResolution, name string, symbol *ast.Symbol) *ast.Symbol {
19734+
if resolution.mapper != nil {
19735+
if _, unaffected := slices.BinarySearch(resolution.unaffected, name); !unaffected {
19736+
return c.newInstantiatedSymbol(symbol, resolution.mapper)
19737+
}
19738+
}
19739+
return symbol
19740+
}
19741+
1973519742
func (c *Checker) everyObjectTypeMemberName(resolution *objectMemberResolution, f func(string) bool) bool {
1973619743
var seen collections.Set[*objectMemberResolution]
1973719744
return c.everyObjectTypeMemberNameWorker(resolution, &seen, f)
@@ -19764,18 +19771,62 @@ func (c *Checker) everyObjectTypeMemberNameWorker(resolution *objectMemberResolu
1976419771
}
1976519772

1976619773
func (c *Checker) resolveObjectTypeProperties(t *Type, resolution *objectMemberResolution) {
19774+
if resolution.mapper != nil && len(resolution.baseTypes) == 0 && !resolution.fullOnly {
19775+
// Canonical declared properties need no temporary name table. Retain any
19776+
// symbols already returned by selective lookups.
19777+
properties := c.getNamedMembers(resolution.source.declaredMembers, t.symbol)
19778+
count := 0
19779+
for _, symbol := range properties {
19780+
name := symbol.Name()
19781+
member, cached := resolution.members[name]
19782+
if !cached {
19783+
member = c.instantiateObjectTypeMember(resolution, name, symbol)
19784+
}
19785+
if member != nil {
19786+
properties[count] = member
19787+
count++
19788+
}
19789+
}
19790+
data := t.AsStructuredType()
19791+
data.properties = properties[:count]
19792+
data.members = c.newPropertyIndex(data.properties, nil)
19793+
t.objectFlags |= ObjectFlagsMembersResolved
19794+
delete(c.objectMemberResolutions, t)
19795+
return
19796+
}
1976719797
if resolution.mapper == nil && len(resolution.baseTypes) == 0 {
1976819798
resolution.members = resolution.source.declaredMembers
1976919799
} else {
1977019800
if resolution.members == nil {
1977119801
resolution.members = make(ast.SymbolTable, len(resolution.source.declaredMembers))
1977219802
}
19773-
for name := range resolution.source.declaredMembers {
19774-
c.resolveObjectTypeMember(resolution, name, true /*instantiate*/)
19803+
for name, symbol := range resolution.source.declaredMembers {
19804+
if _, cached := resolution.members[name]; cached {
19805+
continue
19806+
}
19807+
if symbol.Flags()&ast.SymbolFlagsValue != 0 && (resolution.mapper == nil || !isReservedMemberName(name)) {
19808+
resolution.members[name] = c.instantiateObjectTypeMember(resolution, name, symbol)
19809+
} else {
19810+
c.resolveObjectTypeMember(resolution, name, true /*instantiate*/)
19811+
}
1977519812
}
1977619813
for _, baseType := range resolution.baseTypes {
19777-
for _, property := range c.getPropertiesOfType(baseType) {
19778-
c.resolveObjectTypeMember(resolution, property.Name(), true /*instantiate*/)
19814+
properties := c.getPropertiesOfType(baseType)
19815+
base := c.getReducedApparentType(baseType)
19816+
canonical := base.flags&TypeFlagsObject != 0 && base.objectFlags&ObjectFlagsMembersResolved != 0 &&
19817+
(base.AsStructuredType().members == nil || base.AsStructuredType().members.fallback == nil)
19818+
for _, property := range properties {
19819+
name := property.Name()
19820+
if _, cached := resolution.members[name]; cached {
19821+
continue
19822+
}
19823+
// Enumeration already supplies the first inherited value. Reuse
19824+
// it instead of looking the name up through every base again.
19825+
if canonical && property.Flags()&ast.SymbolFlagsValue != 0 && !isStaticPrivateIdentifierProperty(property) {
19826+
resolution.members[name] = property
19827+
} else {
19828+
c.resolveObjectTypeMember(resolution, name, true /*instantiate*/)
19829+
}
1977919830
}
1978019831
}
1978119832
}

‎tsc/internal/checker/checker_test.go‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -253,6 +253,64 @@ instance;`,
253253
assert.Assert(t, c.GetPropertyOfType(typ, "different") == nil)
254254
}
255255

256+
func TestInstantiatedMemberEnumeration(t *testing.T) {
257+
t.Parallel()
258+
var source strings.Builder
259+
source.WriteString("interface Base<T> { shared: T; ")
260+
for i := range 64 {
261+
fmt.Fprintf(&source, "p%d: T; ", i)
262+
}
263+
source.WriteString("}\ninterface First<T> extends Base<T> { first: T }\n")
264+
source.WriteString("interface Second<T> { shared: T; second: T }\n")
265+
source.WriteString("interface Derived<T> extends First<T>, Second<T> { own: T }\n")
266+
for _, test := range []struct {
267+
name string
268+
typeName string
269+
count int
270+
first string
271+
partial bool
272+
}{
273+
{name: "declared", typeName: "Base", count: 65, first: "shared"},
274+
{name: "declaredCached", typeName: "Base", count: 65, first: "shared", partial: true},
275+
{name: "inherited", typeName: "Derived", count: 68, first: "own"},
276+
{name: "inheritedCached", typeName: "Derived", count: 68, first: "own", partial: true},
277+
} {
278+
t.Run(test.name, func(t *testing.T) {
279+
t.Parallel()
280+
fs := bundled.WrapFS(vfstest.FromMap(map[string]string{
281+
"/main.ts": source.String() + fmt.Sprintf("declare const instance: %s<string>;\ninstance;", test.typeName),
282+
"/tsconfig.json": `{"compilerOptions":{"strict":true},"files":["main.ts"]}`,
283+
}, tspath.CaseInsensitive))
284+
host := compiler.NewCompilerHost(fs, bundled.LibPath(), nil, nil, nil)
285+
parsed, errors := tsoptions.GetParsedCommandLineOfConfigFile("/tsconfig.json", &core.CompilerOptions{}, nil, fs, nil)
286+
assert.Equal(t, len(errors), 0)
287+
program := compiler.NewProgram(compiler.ProgramOptions{Config: parsed, Host: host})
288+
program.BindSourceFiles()
289+
c, done := program.GetTypeChecker(t.Context())
290+
defer done()
291+
file := program.GetSourceFile("/main.ts")
292+
typ := c.GetTypeAtLocation(file.Statements.Nodes[5].Expression())
293+
cached := make(map[string]*ast.Symbol)
294+
if test.partial {
295+
for _, name := range []string{"p63", "shared", "second", "missing"} {
296+
cached[name] = c.GetPropertyOfType(typ, name)
297+
}
298+
}
299+
properties := c.GetPropertiesOfType(typ)
300+
assert.Equal(t, len(properties), test.count)
301+
assert.Equal(t, properties[0].Name(), test.first)
302+
for _, property := range properties {
303+
assert.Equal(t, c.GetPropertyOfType(typ, property.Name()), property)
304+
assert.Equal(t, c.TypeToString(c.GetTypeOfSymbol(property)), "string")
305+
}
306+
for name, symbol := range cached {
307+
assert.Equal(t, c.GetPropertyOfType(typ, name), symbol)
308+
}
309+
assert.Equal(t, len(c.GetDiagnostics(t.Context(), file)), 0)
310+
})
311+
}
312+
}
313+
256314
func TestInstantiatedDiamondMissingMember(t *testing.T) {
257315
t.Parallel()
258316
var source strings.Builder

0 commit comments

Comments
 (0)