-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Add global state storage size logging with threshold filter #3810
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
Conversation
Adds a utility function to log global state storage sizes with a configurable threshold filter (default 10KB) to help users identify large items. This provides visibility into which global state items are consuming significant storage space, making it easier to diagnose performance issues. Fixes: cline#3809 Signed-off-by: Eric Wheeler <[email protected]>
2f7ab2d
to
bcb4268
Compare
@cte @mrubens @hannesrudolph what does It looks like the intention is to prevent files that are not used, but this new file is definitely used (imported by extension.ts) and I cannot figure out how to fix this error:
|
First of all, apologies for my creepy unsolicited entrance. Creator of Knip here. Sometimes I see projects using Knip and this caught my eye. Feel free to ignore me! But I noticed that this is a monorepo and the root {
"$schema": "https://unpkg.com/knip@latest/schema.json",
"workspaces": {
".": {
"entry": ["src/extension.ts", "src/workers/countTokens.ts", "src/**/__tests__/**/*{b,B}enchmark.ts"],
"project": ["src/**/*.ts"]
},
"webview-ui": {
"entry": ["src/index.tsx"],
"project": ["src/**/*.{ts,tsx}"]
}
},
"ignore": ["**/__mocks__/**"]
} This way, there's no need to ignore as much! In the next release of Knip you'll be able to remove that Have a great day ☀️ |
@webpro, Thank you for dropping in! Do you understand what causes the following? As far as I can tell,
|
In a monorepo, the top-level Knip could def need some more config validation, etc. but we're not there yet. Btw the |
thank you I am going to reference this in a new issue |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @KJ7LNW This is great to help us debug potential issues that users might run into with the global state while we fix the implementation, just a nit on the size calculation for the undefined values in the global state, everything else lgtm
stateSizes.forEach((item) => { | ||
if (item.size === -1) { | ||
// Already logged error | ||
} else if (item.size === undefined) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the loop where items are logged, there's a check for item.size === undefined
. If context.globalState.get(key)
actually returns undefined
, JSON.stringify(undefined)
produces the string "undefined"
, so size
would be 9
. Could we clarify how explicitly undefined
values retrieved from globalState
should be handled or reported here? For instance, if a key exists but its value is undefined
, should that be logged differently than an error in size calculation?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting point.
That code is correct, however, as it should measure len(json).toString()
because the size-estimate assumption is that the global state backend storage size is stored as JSON text, so that is the size of the element. (Even if vscode's global state does not store on disk as JSON, it still correctly provides the magnitude of key sizes and the sort order would still be correct as well, and that is all we care about here.)
In any event, the result of a size being only 9 is rather moot: the entire point of this is to provide inspection of large global state sizes, because when the global state gets too big (ie, megabytes) it causes problems, hence PR #3785 and possibly related issues about memory crashes.
This pull request is a result of inspecting global state storage for #3785 to generally see where the problems lie: I thought it could be useful in general for logging Roo startup, to keep an eye on things, and also give users a way to report information like this if troubleshooting is necessary (ie, paste your console logs into this ticket)---however I did not think it should be part of #3785 since that is getting big and I want to keep it focused.
blocked by #3884 |
* Added Cerebras as a Provider * prettier fix * prettier --------- Co-authored-by: sam <[email protected]>
this is a good idea but I do not see it getting off the ground. someone can reopen if they want the feature |
Context
This PR adds a utility function to log global state storage sizes with a configurable threshold filter (default 10KB) to help users identify large items.
Implementation
src/utils/storageUtils.ts
with:formatBytes
function to convert byte sizes to human-readable formatlogGlobalStorageSize
function with threshold parameter (default 10KB)extension.ts
to use the new utility functionsformatBytes
functionconsole.log
instead ofoutputChannel
How to Test
Fixes #3809
Important
Adds utility to log global state storage sizes with a threshold filter and updates logging behavior in the extension.
logGlobalStorageSize
instorageUtils.ts
to log global state items exceeding a size threshold (default 10KB).extension.ts
to uselogGlobalStorageSize
during activation.outputChannel
toconsole.log
inextension.ts
.formatBytes
instorageUtils.ts
to convert byte sizes to human-readable format.formatBytes
instorageUtils.test.ts
.This description was created by
for 2f7ab2d. You can customize this summary. It will automatically update as commits are pushed.