Skip to content

Commit

Permalink
Provide predictable startup ordering (#5959)
Browse files Browse the repository at this point in the history
This change eliminates the disorienting startup experience that can
occur when multiple affiliated runtimes exist (or multiple sessions are
running).

Prior to the change, every runtime start or reconnect resulted in the
new session taking over the Console and Variables panes and becoming the
active session. Practically speaking this meant that startup often
showed some flickering as different sessions booted and Positron
switched between them on its own, and the session you wound up with when
startup was finished might not be the one you wanted or expected.

After the change, sessions and affiliated runtimes are ordered, and only
one console session gets to become active. The other sessions are still
started -- they just start in the background. The result is (hopefully)
a calmer startup experience.

Right now, there is no explicit ordering -- we just go by whichever one
was last used. So if you were using Python when you exited, that's what
you should see when you open Positron again; same deal with R.

The implementation works as follows:

- plumb an `activate` flag everywhere we deal with session start events
to keep track of whether the session should be focused/activated after
it starts
- remember the last used time for both sessions (for reconnect/reload)
and affiliations (for new windows)
- start/reconnect to sessions in descending order of last use (i.e. most
recent first)
- at startup, only flag sessions to `activate` if they are the most
recently used

Addresses #1044 and part of
#1143.

### Release Notes

#### New Features

- When multiple languages are being used, prioritize the most recent one
at startup (#1044.)

#### Bug Fixes

- N/A


### QA Notes

- Both affiliations (for new windows) and sessions (for reloading
existing windows) should be checked.
- For a runtime to become the "last used runtime", you generally need to
make a gesture that you want to use the runtime, such as switching to it
in the Console or running some code in the Console

---------

Signed-off-by: Jonathan <[email protected]>
Co-authored-by: sharon <[email protected]>
Co-authored-by: positron-bot[bot] <173392469+positron-bot[bot]@users.noreply.github.com>
  • Loading branch information
3 people authored Jan 13, 2025
1 parent d625154 commit b0ba2b3
Show file tree
Hide file tree
Showing 17 changed files with 448 additions and 254 deletions.
21 changes: 16 additions & 5 deletions src/vs/workbench/api/browser/positron/mainThreadLanguageRuntime.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ import {
RuntimeInitialState
} from '../../common/positron/extHost.positron.protocol.js';
import { extHostNamedCustomer, IExtHostContext } from '../../../services/extensions/common/extHostCustomers.js';
import { ILanguageRuntimeClientCreatedEvent, ILanguageRuntimeInfo, ILanguageRuntimeMessage, ILanguageRuntimeMessageCommClosed, ILanguageRuntimeMessageCommData, ILanguageRuntimeMessageCommOpen, ILanguageRuntimeMessageError, ILanguageRuntimeMessageInput, ILanguageRuntimeMessageOutput, ILanguageRuntimeMessagePrompt, ILanguageRuntimeMessageState, ILanguageRuntimeMessageStream, ILanguageRuntimeMetadata, ILanguageRuntimeSessionState as ILanguageRuntimeSessionState, ILanguageRuntimeService, ILanguageRuntimeStartupFailure, LanguageRuntimeMessageType, RuntimeCodeExecutionMode, RuntimeCodeFragmentStatus, RuntimeErrorBehavior, RuntimeState, ILanguageRuntimeExit, RuntimeOutputKind, RuntimeExitReason, ILanguageRuntimeMessageWebOutput, PositronOutputLocation, LanguageRuntimeSessionMode, ILanguageRuntimeMessageResult, ILanguageRuntimeMessageClearOutput, ILanguageRuntimeMessageIPyWidget } from '../../../services/languageRuntime/common/languageRuntimeService.js';
import { ILanguageRuntimeSession, ILanguageRuntimeSessionManager, IRuntimeSessionMetadata, IRuntimeSessionService } from '../../../services/runtimeSession/common/runtimeSessionService.js';
import { ILanguageRuntimeClientCreatedEvent, ILanguageRuntimeInfo, ILanguageRuntimeMessage, ILanguageRuntimeMessageCommClosed, ILanguageRuntimeMessageCommData, ILanguageRuntimeMessageCommOpen, ILanguageRuntimeMessageError, ILanguageRuntimeMessageInput, ILanguageRuntimeMessageOutput, ILanguageRuntimeMessagePrompt, ILanguageRuntimeMessageState, ILanguageRuntimeMessageStream, ILanguageRuntimeMetadata, ILanguageRuntimeSessionState as ILanguageRuntimeSessionState, ILanguageRuntimeService, ILanguageRuntimeStartupFailure, LanguageRuntimeMessageType, RuntimeCodeExecutionMode, RuntimeCodeFragmentStatus, RuntimeErrorBehavior, RuntimeState, ILanguageRuntimeExit, RuntimeOutputKind, RuntimeExitReason, ILanguageRuntimeMessageWebOutput, PositronOutputLocation, LanguageRuntimeSessionMode, ILanguageRuntimeMessageResult, ILanguageRuntimeMessageClearOutput, ILanguageRuntimeMessageIPyWidget, RuntimeStartupPhase } from '../../../services/languageRuntime/common/languageRuntimeService.js';
import { ILanguageRuntimeSession, ILanguageRuntimeSessionManager, IRuntimeSessionMetadata, IRuntimeSessionService, RuntimeStartMode } from '../../../services/runtimeSession/common/runtimeSessionService.js';
import { Disposable, DisposableStore } from '../../../../base/common/lifecycle.js';
import { Event, Emitter } from '../../../../base/common/event.js';
import { IPositronConsoleService } from '../../../services/positronConsole/browser/interfaces/positronConsoleService.js';
Expand All @@ -36,7 +36,7 @@ import { Selection } from '../../../../editor/common/core/selection.js';
import { ITextResourceEditorInput } from '../../../../platform/editor/common/editor.js';
import { IPositronDataExplorerService } from '../../../services/positronDataExplorer/browser/interfaces/positronDataExplorerService.js';
import { ISettableObservable, observableValue } from '../../../../base/common/observableInternal/base.js';
import { IRuntimeStartupService, RuntimeStartupPhase } from '../../../services/runtimeStartup/common/runtimeStartupService.js';
import { IRuntimeStartupService } from '../../../services/runtimeStartup/common/runtimeStartupService.js';
import { SerializableObjectWithBuffers } from '../../../services/extensions/common/proxyIdentifier.js';
import { isWebviewReplayMessage } from '../../../contrib/positronWebviewPreloads/browser/utils.js';
import { IPositronWebviewPreloadService } from '../../../services/positronWebviewPreloads/browser/positronWebviewPreloadService.js';
Expand Down Expand Up @@ -105,6 +105,7 @@ class ExtHostLanguageRuntimeSessionAdapter implements ILanguageRuntimeSession {
private readonly _onDidCreateClientInstanceEmitter = new Emitter<ILanguageRuntimeClientCreatedEvent>();

private _currentState: RuntimeState = RuntimeState.Uninitialized;
private _lastUsed: number = Date.now();
private _clients: Map<string, ExtHostRuntimeClientInstance<any, any>> =
new Map<string, ExtHostRuntimeClientInstance<any, any>>();

Expand Down Expand Up @@ -324,6 +325,13 @@ class ExtHostLanguageRuntimeSessionAdapter implements ILanguageRuntimeSession {
return Array.from(this._clients.values());
}

/**
* Returns the last time this session was used (currently, "used" means "executed code")
*/
get lastUsed(): number {
return this._lastUsed;
}

/**
* Convenience method to get the session's ID without having to access the
* the metadata directly.
Expand Down Expand Up @@ -396,6 +404,7 @@ class ExtHostLanguageRuntimeSessionAdapter implements ILanguageRuntimeSession {
}

execute(code: string, id: string, mode: RuntimeCodeExecutionMode, errorBehavior: RuntimeErrorBehavior): void {
this._lastUsed = Date.now();
this._proxy.$executeCode(this.handle, code, id, mode, errorBehavior);
}

Expand Down Expand Up @@ -1171,7 +1180,7 @@ export class MainThreadLanguageRuntime
this._runtimeStartupService.registerMainThreadLanguageRuntime(this._id);

this._disposables.add(
this._runtimeStartupService.onDidChangeRuntimeStartupPhase((phase) => {
this._languageRuntimeService.onDidChangeRuntimeStartupPhase((phase) => {
if (phase === RuntimeStartupPhase.Discovering) {
this._proxy.$discoverLanguageRuntimes();
}
Expand Down Expand Up @@ -1238,7 +1247,9 @@ export class MainThreadLanguageRuntime
sessionName,
sessionMode,
uri,
'Extension-requested runtime selection via Positron API');
'Extension-requested runtime selection via Positron API',
RuntimeStartMode.Starting,
true);

return sessionId;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@

// Other dependencies.
import { IFileDialogService } from '../../../platform/dialogs/common/dialogs.js';
import { ILanguageRuntimeMetadata, ILanguageRuntimeService } from '../../services/languageRuntime/common/languageRuntimeService.js';
import { ILanguageRuntimeMetadata, ILanguageRuntimeService, RuntimeStartupPhase } from '../../services/languageRuntime/common/languageRuntimeService.js';
import { IRuntimeSessionService } from '../../services/runtimeSession/common/runtimeSessionService.js';
import { IRuntimeStartupService, RuntimeStartupPhase } from '../../services/runtimeStartup/common/runtimeStartupService.js';
import { IRuntimeStartupService } from '../../services/runtimeStartup/common/runtimeStartupService.js';
import { EnvironmentSetupType, NewProjectWizardStep, PythonEnvironmentProvider } from './interfaces/newProjectWizardEnums.js';
import { IWorkbenchLayoutService } from '../../services/layout/browser/layoutService.js';
import { IKeybindingService } from '../../../platform/keybinding/common/keybinding.js';
Expand Down Expand Up @@ -163,7 +163,7 @@ export class NewProjectWizardStateManager
this._condaPythonVersionInfo = undefined;
this._minimumRVersion = undefined;

if (this._services.runtimeStartupService.startupPhase === RuntimeStartupPhase.Complete) {
if (this._services.languageRuntimeService.startupPhase === RuntimeStartupPhase.Complete) {
// If the runtime startup is already complete, initialize the wizard state and update
// the interpreter-related state.
this._initDefaultsFromExtensions()
Expand All @@ -174,7 +174,7 @@ export class NewProjectWizardStateManager
} else {
// Register disposables.
this._register(
this._services.runtimeStartupService.onDidChangeRuntimeStartupPhase(
this._services.languageRuntimeService.onDidChangeRuntimeStartupPhase(
async (phase) => {
if (phase === RuntimeStartupPhase.Discovering) {
// At this phase, the extensions that provide language runtimes will
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import { usePositronConsoleContext } from '../positronConsoleContext.js';
import { PositronActionBarContextProvider } from '../../../../../platform/positronActionBar/browser/positronActionBarContext.js';
import { PositronConsoleState } from '../../../../services/positronConsole/browser/interfaces/positronConsoleService.js';
import { RuntimeExitReason, RuntimeState } from '../../../../services/languageRuntime/common/languageRuntimeService.js';
import { ILanguageRuntimeSession } from '../../../../services/runtimeSession/common/runtimeSessionService.js';
import { ILanguageRuntimeSession, RuntimeStartMode } from '../../../../services/runtimeSession/common/runtimeSessionService.js';
import { ConsoleInstanceMenuButton } from './consoleInstanceMenuButton.js';

/**
Expand Down Expand Up @@ -299,7 +299,9 @@ export const ActionBar = (props: ActionBarProps) => {
session.metadata.sessionMode,
session.metadata.notebookUri,
`User-requested new session from console action bar ` +
`after session ${session.metadata.sessionId} exited.`
`after session ${session.metadata.sessionId} exited.`,
RuntimeStartMode.Starting,
false
);
return;
} else {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*---------------------------------------------------------------------------------------------
* Copyright (C) 2023-2024 Posit Software, PBC. All rights reserved.
* Copyright (C) 2023-2025 Posit Software, PBC. All rights reserved.
* Licensed under the Elastic License 2.0. See LICENSE.txt for license information.
*--------------------------------------------------------------------------------------------*/

Expand All @@ -16,7 +16,7 @@ import { EmptyConsole } from './emptyConsole.js';
import { ConsoleInstance } from './consoleInstance.js';
import { usePositronConsoleContext } from '../positronConsoleContext.js';
import { StartupStatus } from './startupStatus.js';
import { RuntimeStartupPhase } from '../../../../services/runtimeStartup/common/runtimeStartupService.js';
import { RuntimeStartupPhase } from '../../../../services/languageRuntime/common/languageRuntimeService.js';

// ConsoleCoreProps interface.
interface ConsoleCoreProps {
Expand All @@ -35,11 +35,11 @@ export const ConsoleCore = (props: ConsoleCoreProps) => {
const positronConsoleContext = usePositronConsoleContext();

const [startupPhase, setStartupPhase] = useState(
positronConsoleContext.runtimeStartupService.startupPhase);
positronConsoleContext.languageRuntimeService.startupPhase);

useEffect(() => {
const disposables =
positronConsoleContext.runtimeStartupService.onDidChangeRuntimeStartupPhase(
positronConsoleContext.languageRuntimeService.onDidChangeRuntimeStartupPhase(
e => {
setStartupPhase(e);
});
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*---------------------------------------------------------------------------------------------
* Copyright (C) 2024 Posit Software, PBC. All rights reserved.
* Copyright (C) 2024-2025 Posit Software, PBC. All rights reserved.
* Licensed under the Elastic License 2.0. See LICENSE.txt for license information.
*--------------------------------------------------------------------------------------------*/

Expand All @@ -14,7 +14,7 @@ import { localize } from '../../../../../nls.js';
import { usePositronConsoleContext } from '../positronConsoleContext.js';
import { ProgressBar } from '../../../../../base/browser/ui/progressbar/progressbar.js';
import { DisposableStore } from '../../../../../base/common/lifecycle.js';
import { RuntimeStartupPhase } from '../../../../services/runtimeStartup/common/runtimeStartupService.js';
import { RuntimeStartupPhase } from '../../../../services/languageRuntime/common/languageRuntimeService.js';

// Load localized copy for control.
const initalizing = localize('positron.console.initializing', "Starting up");
Expand All @@ -41,7 +41,7 @@ export const StartupStatus = () => {
const [discovered, setDiscovered] =
useState(positronConsoleContext.languageRuntimeService.registeredRuntimes.length);
const [startupPhase, setStartupPhase] =
useState(positronConsoleContext.runtimeStartupService.startupPhase);
useState(positronConsoleContext.languageRuntimeService.startupPhase);

useEffect(() => {
const disposableStore = new DisposableStore();
Expand All @@ -65,7 +65,7 @@ export const StartupStatus = () => {

// When the startup phase changes, update the phase.
disposableStore.add(
positronConsoleContext.runtimeStartupService.onDidChangeRuntimeStartupPhase(
positronConsoleContext.languageRuntimeService.onDidChangeRuntimeStartupPhase(
phase => {
setStartupPhase(phase);
}));
Expand Down
Original file line number Diff line number Diff line change
@@ -1,16 +1,17 @@
/*---------------------------------------------------------------------------------------------
* Copyright (C) 2023-2024 Posit Software, PBC. All rights reserved.
* Copyright (C) 2023-2025 Posit Software, PBC. All rights reserved.
* Licensed under the Elastic License 2.0. See LICENSE.txt for license information.
*--------------------------------------------------------------------------------------------*/

import * as nls from '../../../../nls.js';
import { Emitter } from '../../../../base/common/event.js';
import { Event, Emitter } from '../../../../base/common/event.js';
import { ILogService } from '../../../../platform/log/common/log.js';
import { Disposable, IDisposable, toDisposable } from '../../../../base/common/lifecycle.js';
import { InstantiationType, registerSingleton } from '../../../../platform/instantiation/common/extensions.js';
import { ILanguageRuntimeMetadata, ILanguageRuntimeService, formatLanguageRuntimeMetadata } from './languageRuntimeService.js';
import { ILanguageRuntimeMetadata, ILanguageRuntimeService, RuntimeStartupPhase, formatLanguageRuntimeMetadata } from './languageRuntimeService.js';
import { Registry } from '../../../../platform/registry/common/platform.js';
import { IConfigurationRegistry, Extensions as ConfigurationExtensions, ConfigurationScope, IConfigurationNode, } from '../../../../platform/configuration/common/configurationRegistry.js';
import { ISettableObservable, observableValue } from '../../../../base/common/observableInternal/base.js';

/**
* The implementation of ILanguageRuntimeService
Expand All @@ -26,6 +27,9 @@ export class LanguageRuntimeService extends Disposable implements ILanguageRunti
private readonly _onDidRegisterRuntimeEmitter =
this._register(new Emitter<ILanguageRuntimeMetadata>);

// The current startup phase; an observeable value.
private _startupPhase: ISettableObservable<RuntimeStartupPhase>;

//#endregion Private Properties

//#region Constructor
Expand All @@ -40,6 +44,19 @@ export class LanguageRuntimeService extends Disposable implements ILanguageRunti
) {
// Call the base class's constructor.
super();

this._startupPhase = observableValue(
'runtime-startup-phase', RuntimeStartupPhase.Initializing);
this.onDidChangeRuntimeStartupPhase = Event.fromObservable(this._startupPhase);
}

/**
* Sets the startup phase
*
* @param phase The new phase
*/
setStartupPhase(phase: RuntimeStartupPhase): void {
this._startupPhase.set(phase, undefined);
}

//#endregion Constructor
Expand All @@ -52,6 +69,11 @@ export class LanguageRuntimeService extends Disposable implements ILanguageRunti
// An event that fires when a new runtime is registered.
readonly onDidRegisterRuntime = this._onDidRegisterRuntimeEmitter.event;

/**
* Event tracking the current startup phase.
*/
onDidChangeRuntimeStartupPhase: Event<RuntimeStartupPhase>;

/**
* Gets the registered runtimes.
*/
Expand Down Expand Up @@ -117,6 +139,13 @@ export class LanguageRuntimeService extends Disposable implements ILanguageRunti
return this._registeredRuntimesByRuntimeId.get(runtimeId);
}

/**
* Returns the current startup phase.
*/
get startupPhase(): RuntimeStartupPhase {
return this._startupPhase.get();
}

//#endregion ILanguageRuntimeService Implementation
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*---------------------------------------------------------------------------------------------
* Copyright (C) 2023-2024 Posit Software, PBC. All rights reserved.
* Copyright (C) 2023-2025 Posit Software, PBC. All rights reserved.
* Licensed under the Elastic License 2.0. See LICENSE.txt for license information.
*--------------------------------------------------------------------------------------------*/

Expand Down Expand Up @@ -540,6 +540,51 @@ export enum LanguageRuntimeStartupBehavior {
Manual = 'manual'
}

/**
* The phases through which the runtime startup service progresses as Positron
* starts.
*/
export enum RuntimeStartupPhase {
/**
* Phase 1: The startup sequence has not yet begun.
*/
Initializing = 'initializing',

/**
* Phase 2: If the workspace is not trusted, we cannot proceed with startup,
* since many runtimes run arbitrary code at startup (often from the
* workspace contents) and we cannot trust them to do so safely. The startup
* sequence stays at `AwaitingTrust` until workspace trust is granted.
*/
AwaitingTrust = 'awaitingTrust',

/**
* Phase 3: Positron is reconnecting to runtimes that are already running.
* We only enter this phase when reloading the UI, or when reopening a
* browser tab.
*/
Reconnecting = 'reconnecting',

/**
* Phase 4: Positron is starting any runtimes that are affiliated with the
* workspace. We enter this phase on a fresh start of Positron, when no
* existing sessions are running.
*/
Starting = 'starting',

/**
* Phase 5: Positron is discovering all the runtimes on the machine. This
* can take a while, but does precede startup for workspaces that have no
* affiliated runtimes (so we don't know what to start yet).
*/
Discovering = 'discovering',

/**
* Phase 6: Startup is complete. In this phase, we start any runtimes
* recommended by extensions if nothing was started in previous phases.
*/
Complete = 'complete',
}

export interface ILanguageRuntimeMessageState extends ILanguageRuntimeMessage {
/** The new state */
Expand Down Expand Up @@ -661,9 +706,16 @@ export interface ILanguageRuntimeService {
// Needed for service branding in dependency injector.
readonly _serviceBrand: undefined;

// An event that fires when a new runtime is registered.
/**
* An event that fires when a new runtime is registered.
*/
readonly onDidRegisterRuntime: Event<ILanguageRuntimeMetadata>;

/**
* Event tracking the current startup phase.
*/
onDidChangeRuntimeStartupPhase: Event<RuntimeStartupPhase>;

/**
* Gets the registered language runtimes.
*/
Expand Down Expand Up @@ -691,5 +743,17 @@ export interface ILanguageRuntimeService {
* @param runtimeId The ID of the runtime to unregister
*/
unregisterRuntime(runtimeId: string): void;

/**
* Sets the current startup phase.
*
* @param phase The new startup phase
*/
setStartupPhase(phase: RuntimeStartupPhase): void;

/**
* Returns the current startup phase.
*/
get startupPhase(): RuntimeStartupPhase;
}

Loading

0 comments on commit b0ba2b3

Please sign in to comment.