perf(typescript-language): stop tsserver formatting discarded log messages - #3256
Draft
JoshuaKGoldberg wants to merge 1 commit into
Draft
perf(typescript-language): stop tsserver formatting discarded log messages#3256JoshuaKGoldberg wants to merge 1 commit into
JoshuaKGoldberg wants to merge 1 commit into
Conversation
…sages Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
✅ Deploy Preview for flint-fyi ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
SASSAFRAS
PR Checklist
status: accepting prsOverview
@typescript-eslint/project-servicebuilds its tsserver logger withhasLevel: () => truewhileloggingEnabled()correctly reports whether anydebugnamespace is on. tsserver useshasLevel()as the guard before formatting a message, so with noDEBUGnamespaces enabled it still builds every internal log string and hands it to a disableddebugnamespace.There is no supported seam for this:
createProjectService's settings are{ host, jsDocParsingMode, options, tsconfigRootDir },ProjectServiceOptionshas no logger field, and thehostwe pass becomes tsserver'sServerHost, not its logger.ts.server.ProjectService#loggerandts.server.Logger#hasLevelare both public TypeScript API, though, so the returned service'shasLevelcan be pointed at theloggingEnabled()it should have agreed with in the first place:With any of the
typescript-eslint:project-service:tsserver:*namespaces enabled this is a no-op, so debugging tsserver still works exactly as before. With them disabled, tsserver skips the guarded logging entirely.The win is larger than the issue estimated, because the issue counted only
msganddebugself time. The dominant cost isProjectService#printProjects, which is gated onhasLevel(LogLevel.normal)and is called after everyopenClientFile/closeClientFile— so on a 1024-file run it walks every open file and every project ~2000 times, building strings nobody reads.CPU profile of
files-1024-rules-many, inclusive/self time for the affected frames:printProjects(typescript.js)msg(createProjectService.js)info(createProjectService.js)debug(debug/common.js)Measurements
hyperfine -N --warmup 2 -r 8, back-to-back on the same machine, using the #3024 fixture cases.Warm cache, same runs:
Findings are byte-identical to
mainonfiles-256-rules-manyandfiles-1024-rules-many(--cache-ignore --skip-formatting --skip-language-reports).Not covered
@typescript-eslint/project-service: insrc/createProjectService.ts, the logger'shasLevel: () => trueshould defer to the same namespace checkloggingEnabledalready does (logTsserverInfo.enabled || logTsserverErr.enabled || logTsserverPerf.enabled). The existing comment there explains why it was left astrue—debughas no levels — but "no levels" only justifies not distinguishing levels, not claiming a level is enabled when no namespace is. EveryhasLevelcall site in tsserver guards logging only, so agreeing withloggingEnabledcosts nothing but the same all-or-nothing granularity the writer already has. Once that ships, this line should be deleted.ts.server.ProjectServiceitself and own the logger outright, which would also let it drop the dependency. That means taking on the stubbed system, plugin blocking,setHostConfiguration, and thedefaultProject→setCompilerOptionsForInferredProjectspath (which needs@typescript-eslint/tsconfig-utils'getParsedConfigFilereimplemented). That felt like a lot of surface to own for a logging flag, so I did not do it.createFileFactoryhas no test file today and the behavior is "tsserver does less work", which is only observable in a profile.