API usage guidelines

[DonJayamanne]

It feels like accessing the known property can result in a lot of code getting executed, even if its just logging (i.e. 1,500 entries in a few seconds is a lot of logging).

Here #22993

Also looking at the code in Python extension

        get known(): Environment[] {
            sendApiTelemetry('known');
            return discoveryApi
                .getEnvs()
                .filter((e) => filterUsingVSCodeContext(e))
                .map((e) => convertEnvInfoAndGetReference(e));
        },

There's a lot going on here, hence it feels like I (as a a user of the above API) would need to be careful accessing this 100s of times.
As this could unnecessarily chew CPU,

Question

• Should we (consumers of the API) cache the value of known
• Is it possible for Python extnsion to instead cache the known value into a property, after all there are events for triggering the changes, hence that feels like its doable.

Anyways, asking this as I fear that changes in Jupyter to now access know everytime we need somethign is actually going to slow the extnsion host significantly as theres a lot going on here,
I.e. calling this 100s of times in a few ms could cause issues (after all this is what I was experiencing)

Note:

• When I opened the Jpuyter kernel picker and attempted to select a Python environment, I did notice some sluggishness in the UI
• Possible it was unrelated, but I thought of checking the logs and then saw the logging and then looked at the API implementation, hence this issue

[DonJayamanne]

I have found that accessing environment.known and getting the env details by searching this is very slow.
Basically Jupyter extension is almost 10 times slower in finding kernels now.
In stable version kernel picker is populated in 4 seconds.
In the latest versionn with the changes to query environments.known instead of using our own cached object, it now takes >30s

[karrtikr]

Thanks for all the investigation.

environments.known is indeed misleading as a property in the external API type, when it really should probably be a function so it indicates that it is not as cheap. It currently:

1. Figures out caller extension from stack in background
2. Sends API usage telemetry
3. Converts legacy environments types to external API types
4. Filtering

Based on offline testing 1. and 3. are the main bottlenecks, which should be solved with #22998 and #20240 respectively for the long term. 3 requires a big change, so until we get to it, currently every call to environments.known still takes 1 ms, which can be significant if there are 10000 calls.

Regarding API guidelines,

• environments.known should not be called every time an event is triggered, instead events can be used to build the list. In the Python extension, environments.known is used exactly once, when opening the list, and any events are later used to add/update items to that list, with help of the id property. That's what we would request other extensions to follow as well for best perf.
• resolveEnvironment: These practices should be followed:
  • Make sure to use resolveEnvironment for the selected Python kernel to get version information vscode-jupyter#15254
  • Explore not resolving an environment using Python API unless selected vscode-jupyter#15140: still seems to be an issue.
• refreshEnvironment: Avoid calling unless explicitly triggered by user: Do not automatically trigger environment discovery at startup vscode-jupyter#12719

Let me know if you have any questions.

[DonJayamanne]

I think we should reopen this
As the solution might now be different if changes are going to be made to python extension to improve the performance (remove previously mentioned bottle necks)

Else someone looking at this issue world try this solution which might now be right

[karrtikr]

Please let me know once calling extension id PR is fixed and I can add the link to updated guidelines here, in case someone runs into the issue. Until then the guideline stated in the issue is true.