Skip to content
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

localStorage access is slow #18439

Closed
jrieken opened this issue Jan 12, 2017 · 7 comments
Closed

localStorage access is slow #18439

jrieken opened this issue Jan 12, 2017 · 7 comments
Assignees
Labels
debt Code quality issues perf perf-startup workbench-state UI state across restarts

Comments

@jrieken
Copy link
Member

jrieken commented Jan 12, 2017

  • scripts\code.sh
  • hide viewlet
  • close all editors
  • restart profiler, reload, stop profiler
  • 🐌 you see 135ms spend in index.html reading the theme from local storage (I assume the times vary with the size/history of the workspace)

screen shot 2017-01-12 at 10 56 47

CPU-20170112T105633.cpuprofile.zip

@jrieken jrieken added the perf label Jan 12, 2017
@bpasero bpasero added the upstream Issue identified as 'upstream' component related (exists outside of VS Code) label Jan 12, 2017
@bpasero bpasero added this to the Backlog milestone Jan 12, 2017
@bpasero
Copy link
Member

bpasero commented Jan 12, 2017

For more reasons than just performance, local storage should be file based.

@jrieken
Copy link
Member Author

jrieken commented Jan 12, 2017

How is this upstream? Surly local storage is file based (how else should it work) the problem is that we need to load all of local storage to read a single value. That the local storage API and I don't see that changing. I think we should just stop using locale storage and implement our own storage mechanism.

@bpasero bpasero modified the milestones: Backlog, On Deck Sep 16, 2017
@jrieken
Copy link
Member Author

jrieken commented Nov 2, 2017

screen shot 2017-11-02 at 17 32 23

StorageService#setWorkspaceId "nicely" exposes this and blocks startup

@bpasero bpasero added workbench-state UI state across restarts and removed workbench upstream Issue identified as 'upstream' component related (exists outside of VS Code) labels Nov 16, 2017
@bpasero bpasero added the debt Code quality issues label Nov 23, 2017
@jrieken
Copy link
Member Author

jrieken commented Dec 1, 2017

Simple experiment I ran is to start with code-insiders --prof-startup, one time with the LocalStorage folder as it is today (for me ~9MB) and once with a fresh, empty folder. Difference is around ~150ms.

I'd say this one of the last low hanging items that can save a significant chunk of startup performance

with data
screen shot 2017-12-01 at 16 33 32

without data
screen shot 2017-12-01 at 16 36 16

@jrieken
Copy link
Member Author

jrieken commented Dec 1, 2017

@bpasero Unrelated question: what does cleanupWorkspace do and why does it run every time I open a workspace and why does everything need to wait for this?

@bpasero
Copy link
Member

bpasero commented Dec 1, 2017

@jrieken I think it is just the first guy who uses local storage that pays the price of it. If I remove that call, the next one hits it right after:

image

The cleanup is useful in cases where a folder was deleted and then recreated with the same name. We store the ctime of a folder in local storage and compare it to the ctime of the folder that is opened. If the path is the same but the ctime different, we clear all storage for that workspace early on assuming that the folder was deleted and recreated.

@bpasero
Copy link
Member

bpasero commented Dec 10, 2018

Via #64719, we are not using window.localStorage anymore.

@bpasero bpasero closed this as completed Dec 10, 2018
@vscodebot vscodebot bot locked and limited conversation to collaborators Jan 24, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
debt Code quality issues perf perf-startup workbench-state UI state across restarts
Projects
None yet
Development

No branches or pull requests

2 participants