Theia: [preferences][multi root] Potentially exagerated number of calls to getConfigUri()

[marcdumais-work]

Description

Originally reported in this comment: https://github.com/eclipse-theia/theia/pull/7105#issuecomment-616695766

on current master, just loading the application in a multi-root workspace seems to call the getConfigUri method in the abstract preference provider (packages/preferences/src/browser/abstract-resource-preference-provider.ts) more than 14,000 times. That may be necessary, but it seems like there may be a more efficient way to build the preference object.

14000 times seems excessive.

@akosyakov 's suggestion:

Someone has to measure whether there is really an issue. You could file an issue with a request to investigate it. If it is not hot spot I would prefer don't touch it. Our preference resolution logic is tricky matter.

Reproduction Steps

OS and Theia version:
App: browser example
OS: RHEL 7
Theia version: latest master

Diagnostics:

[colin-grant-work]

I can add that OS was RHEL 7 running the browser example app.

[elaihau]

i found a few things:

• most getConfigUri() are made from FolderPreferenceProvider, and every root folder in the workspace has 6 FolderPreferenceProviders, i.e., 2 for tasks.json, 2 for launch.json, & 2 for settings.json.

• caching config uri is not an easy job, as the return value of getConfigUri() is dependent on the status of the file (i.e., tasks.json, launch.json, & settings.json), root folders in the current workspace, and the resourceUri passed into the function. Around 95% of the time, getConfigUri() is called with a resourceUri.

• surprisingly getConfigUri() does not take much time. I added a timer to track the total time spent on the running the function. none of my 6 providers spent more than 10 ms after I finished the following operations

  • reloading theia by hitting the refresh button of my Chrome browser
  • open the preference editor

[akosyakov]

We should investigate first whether it causes any bad side effects. If not there is nothing to be done. Particularly in each profiling sessions check whether it is the hottest call.

[colin-grant-work]

Anecdotally, I don't think it really slowed anything down until I tried logging it, and then it was almost certainly the logging that slowed things down, not the logic. But this may also be a chance for me to understand part of this system better. @elaihau , you mention the six providers for each folder; could you help me understand why the launch.json and tasks.json are counted as preference providers along with the settings.json, and why only for the purposes of the folder preference provider?

[elaihau]

you mention the six providers for each folder; could you help me understand why the launch.json and tasks.json are counted as preference providers along with the settings.json,

I think we access data in launch.json and tasks.json from Preferences mostly due to the goal of achieving vscode compatibility:
Please see the descriptions in https://github.com/eclipse-theia/theia/pull/4947, and https://github.com/eclipse-theia/theia/pull/6268

The reason that we have 6 providers per root folder is because, not only do we get data from .theia folder but also from .vscode. I believe the purpose behind this, is helping vscode users to more seamlessly migrate from using vscode to theia.

and why only for the purposes of the folder preference provider?

i am not sure if i misunderstand you question.
in a multi root workspace, tasks.json and settings.json are stored under a root folder, and data in those files doesn't have any impact on other root folders in the same workspace.
sorry I can't say anything about launch.json as i have never touched it :)

it was almost certainly the logging that slowed things down, not the logic

Yep i think so too. so in my experiment i don't log anything until the accumulated run time of that function exceeds a threshold, say, 5 ms.

[colin-grant-work]

@elaihau, thanks that's very helpful!

[elaihau]

We should investigate first whether it causes any bad side effects. If not there is nothing to be done. Particularly in each profiling sessions check whether it is the hottest call.

@akosyakov
As per the profiling tool in firefox, this function takes very little time.

From static analysis, I can see that this function is called every time a preference item is retrieved, updated, or initilized. It might have slowed down the startup a little, but I don't think it has major impacts as it doesn't block anything.

[akosyakov]

ok, let's move on then

Just think about React virtual DOM rendering i'm pretty sure that there are much many calls to our render method, we are not going to rewrite react because of it.

[elaihau]

If there are no objections I am going to close this one in a day.