Defer positron-reticulate activation until needed#13086
Defer positron-reticulate activation until needed#13086juliasilge wants to merge 2 commits intomainfrom
positron-reticulate activation until needed#13086Conversation
|
E2E Tests 🚀 |
| "lint": "eslint src --ext ts" | ||
| }, | ||
| "contributes": { | ||
| "languageRuntimes": [ |
There was a problem hiding this comment.
This was the reason the extension was eagerly activated during discoverAllRuntimes, but the languageId: "reticulate" anchor was a no-op (the actual runtime metadata registers under languageId: 'python').
| } | ||
| } | ||
| }, | ||
| "extensionDependencies": [ |
There was a problem hiding this comment.
These forced the Python and R extensions to fully activate before reticulate's own activate() could even start, but that's not necessary.
| } | ||
|
|
||
| createPythonRuntimeSession(runtimeMetadata: positron.LanguageRuntimeMetadata, sessionMetadata: positron.RuntimeSessionMetadata, kernelSpec?: JupyterKernelSpec): positron.LanguageRuntimeSession { | ||
| async createPythonRuntimeSession(runtimeMetadata: positron.LanguageRuntimeMetadata, sessionMetadata: positron.RuntimeSessionMetadata, kernelSpec?: JupyterKernelSpec): Promise<positron.LanguageRuntimeSession> { |
There was a problem hiding this comment.
We're making this async and explicitly await-ing api.activate() when the Python extension isn't yet active (needed now because, without extensionDependencies, api.exports is only populated post-activation).
| this.onDidUpdateResourceUsage = this._resourceUsageEmitter.event; | ||
| } | ||
|
|
||
| private async initializePythonSession(runtimeMetadata: positron.LanguageRuntimeMetadata): Promise<void> { |
There was a problem hiding this comment.
This is new, to split out the async work out of the ReticulateRuntimeSession constructor. The create and restore factories now await session.initializePythonSession(metadata) after construction. The restart() path's createPythonRuntimeSession call is also now awaited.
|
With these changes, I do see the reticulate startup time drop to 3ms. But reviewing the startup diagnostics, with a simple workspace containing a single session with an affiliated version of R, I still see Workspace-Affiliated Runtimes
Active Sessions
Time to First Runtime Ready
Startup Phase Progression
Positron Runtime Performance Marks
|
|
Gosh, that is very surprising, @jmcphers! 😱 I definitely do not see that locally for myself. Here's a fresh startup diagnostic I just captured from the current state of this branch (after the merge from
The This PR removes that contribution from reticulate's manifest (see My best guess is a stale extension-description cache on your side is still listing the old Would you mind pulling the merge, doing a full rebuild, and re-running diagnostics to see if reticulate is still showing up for you? If it is, there's another path into |
2 participants
Addresses #12478
After #12645,
positron-reticulateactivation no longer blocks time-to-first-console, but the extension was still being activated during the runtime-discovery phase on every startup, regardless of whether the user ever used reticulate. This PR defers that activation toonStartupFinishedand drops theextensionDependencieschain that was forcingms-python.pythonandpositron.positron-rto be pulled in by reticulate's activation path.The extension still activates on
onStartupFinishedso the client handler forpositron.reticulatecomms, the config-change listener, and the runtime manager registration are all in place before the user can realistically triggerreticulate::repl_python(). For persisted machine-level sessions,restoreWorkspaceSessionsactivates reticulate by extension id, which doesn't require alanguageRuntimescontribution.Runtime startup marks (before vs. after)
From just a single comparison on a local dev build opening the same workspace with R and Python runtimes available:
runtimeStartup/extensionPreActivate/positron.positron-reticulateruntimeStartup/extensionPostActivate/positron.positron-reticulateonLanguageRuntime:reticulateonStartupFinishedThe removal of reticulate from the runtime-discovery activation set is the structural change this PR guarantees. The
firstRuntimeReadyandruntimeStartupPhase/completemarks also landed earlier on the branch FWIW.Release Notes
New Features
Bug Fixes
positron-reticulateactivation until it is actually needed, reducing unnecessary work during Positron startup (Startup performance: deferpositron-reticulateactivation until needed #12478)QA Notes
@:reticulate
positron.positron-reticulateshowsonStartupFinished(notonLanguageRuntime:reticulate), with a small activation time, andBy: positron.positron-reticulate.reticulate::repl_python()and confirm a Python (reticulate) session starts as before.kernelSupervisor.shutdownTimeoutto something other thanimmediately, start a reticulate session, reload Positron, and confirm the session is restored (exercises the restore path that now relies onactivateById).positron.reticulate.enabled = "always"in a workspace with R available, reload, and confirm the reticulate runtime registers.