microsoft / vscode-jupyter

VS Code Jupyter extension
https://marketplace.visualstudio.com/items?itemName=ms-toolsai.jupyter
MIT License
1.29k stars 290 forks source link

Tensorboard file watcher causes high CPU load #8301

Closed roblourens closed 2 years ago

roblourens commented 2 years ago

image

image

I noticed that the python extension (and jupyter) use findFiles to start a search when the extension is activated. Extensions running searches is one of the top things that leads to people saying that vscode is "slow" or "bloated" so we really want to avoid this.

https://github.com/microsoft/vscode-python/blob/344e98262ae39d064ff8c921ad499d524f433be1/src/client/tensorBoard/tensorBoardFileWatcher.ts#L59

Not sure what this is used for, but do you absolutely have to have that info? If so, maybe you could limit the search to run for 5s.

https://github.com/microsoft/vscode-jupyter/issues/8293

joyceerhl commented 2 years ago

That code is used to promote discoverability of the TensorBoard feature, it searches a max of two directories deep and puts up a toast when it finds a tfevent file. We have it looking two directories down because that's how far down tfevent files get created in TensorBoard tutorial code. I think we should just take out the file watcher, the perf hit isn't worth the discoverability IMO. Probably also a good time to check in on feature usage

greazer commented 2 years ago

Looking at the code, while the depth is limited, looking 2 directories deep can still result in many dirs being searched. It would be great if we could know how many users are prompted for tensorboard support because we found a tfevent file 0, 1 or 2 dirs deep. Doesn't seem like that information is readily available.

So the ONLY reason we're even doing this is to essentially let users know that tensorboard features exist, right? I.e. knowing where we found event files has no bearing on how the feature works. Is that correct? If so, then yes, it would seem that removing this search altogether would be fine.

roblourens commented 2 years ago

Limiting the depth will not actually help with perf currently: https://github.com/microsoft/vscode/issues/137484 there is also an issue to revamp findFiles to add things like a maxDepth flag. But that can still lead to looking at lots of files, like you poitn out.