Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 22 additions & 1 deletion packages/@ember/-internals/glimmer/lib/base-renderer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import type { SimpleDocument, SimpleElement } from '@simple-dom/interface';
import { hasDOM } from '../../browser-environment';
import { EmberEnvironmentDelegate } from './environment';
import ResolverImpl from './resolver';
import { _registeredStrategy } from '@ember/scheduler';
import { EvaluationContextImpl } from '@glimmer/opcode-compiler/lib/program-context';

export type IBuilder = (env: Environment, cursor: Cursor) => TreeBuilder;
Expand Down Expand Up @@ -368,8 +369,28 @@ export class RendererState {
}
}

#renderer: BaseRenderer | null = null;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not believe we are done.

Because we haven't wired up all our internals to the scheduler, a user swapping out the scheduler would see no benefit.

#21520 made a lot of progress on that work, and you'll need to copy some of that here (though, only insofar as it applies to enabling the classic strategy.

This is also way too much code.

Read https://github.com/runspired/rfcs/blob/modernized-scheduler/text/0957-modernized-scheduler.md again


// stable identity so strategies (and classic scheduleOnce dedupe) can
// coalesce repeat scheduling between flushes
#revalidateCurrent = (): void => {
if (this.#renderer !== null) {
this.revalidate(this.#renderer);
}
};

scheduleRevalidate(renderer: BaseRenderer): void {
_backburner.scheduleOnce('render', this, this.revalidate, renderer);
this.#renderer = renderer;

const strategy = _registeredStrategy;

if (strategy !== null && strategy._scheduleRevalidate !== undefined) {
strategy._scheduleRevalidate(this.#revalidateCurrent);
} else {
// pre-boot (no strategy registered yet) or a registered strategy
// without the internal seam: classic runloop scheduling
_backburner.scheduleOnce('render', this, this.revalidate, renderer);
}
}

isValid(): boolean {
Expand Down
9 changes: 9 additions & 0 deletions packages/@ember/-internals/glimmer/lib/environment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import type { DeprecationOptions } from '@ember/debug/lib/deprecate';
import { schedule, _backburner } from '@ember/runloop';
import { DEBUG } from '@glimmer/env';
import setGlobalContext from '@glimmer/global-context';
import { registerStrategy } from '@ember/scheduler';
import classicStrategy from '@ember/scheduler/-private/classic';
import type { EnvironmentDelegate } from '@glimmer/runtime/lib/environment';
import { debug } from '@glimmer/validator/lib/debug';
import toIterator from './utils/iterator';
Expand All @@ -17,6 +19,13 @@ import toBool from './utils/to-bool';

///////////

// The glimmer<->ember hookup is where the framework's scheduling
// strategy is established: classic (today's runloop scheduling) unless
// an application swaps it via `registerStrategy`.
registerStrategy(classicStrategy);

///////////

// Setup global context

setGlobalContext({
Expand Down
63 changes: 63 additions & 0 deletions packages/@ember/scheduler/-private/classic.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
import { _backburner, next as runloopNext, schedule } from '@ember/runloop';
import type { Strategy } from '@ember/scheduler';

/**
* The ambient default strategy: schedules exactly the way Ember works
* today, so existing applications observe no change in timing. Phases
* map onto the runloop's queues (`render`, then `afterRender`, with
* `composite` re-scheduled behind `layout`'s queue entries within the
* same flush), and the renderer's revalidation is a
* `scheduleOnce('render', ...)`, just as it always was.
*
* Render-aware scheduling (frame-aligned phases, coalesced
* revalidation) is what a swapped-in strategy provides -- see
* `@ember/scheduler/strategy` -- and becomes the source of performance
* wins when it becomes the default.
*
* @internal
*/
class ClassicStrategy implements Strategy {
render(): Promise<void> {
return new Promise((resolve) => schedule('render', null, resolve));
}

layout(): Promise<void> {
return new Promise((resolve) => schedule('afterRender', null, resolve));
}

composite(): Promise<void> {
return new Promise((resolve) =>
schedule('afterRender', null, () => schedule('afterRender', null, resolve))
);
}

next(): Promise<void> {
return new Promise((resolve) => runloopNext(null, resolve));
}

idle(): Promise<void> {
return new Promise((resolve) => {
if (typeof requestIdleCallback === 'function') {
// fully-idle or backgrounded pages can starve requestIdleCallback
// indefinitely; cap the wait to keep the promise resolvable
requestIdleCallback(() => resolve(), { timeout: 500 });
} else {
setTimeout(resolve, 0);
}
});
}

/**
* The renderer's internal seam: how revalidation gets scheduled.
* Classic behavior is a runloop `scheduleOnce`, preserving today's
* timing exactly (the flush callback is stable per renderer, so
* scheduleOnce's dedupe applies as before).
*/
_scheduleRevalidate(flush: () => void): void {
_backburner.scheduleOnce('render', null, flush);
}
}

const classicStrategy: ClassicStrategy = new ClassicStrategy();

export default classicStrategy;
68 changes: 51 additions & 17 deletions packages/@ember/scheduler/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { assert } from '@ember/debug';
import classicStrategy from '@ember/scheduler/-private/classic';

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should not import this here.


/**
The `@ember/scheduler` package provides a render-aware scheduling interface,
Expand Down Expand Up @@ -67,9 +68,28 @@ export interface Strategy {
composite(): Promise<void>;
next(): Promise<void>;
idle(): Promise<void>;

/**
* Internal seam used by the renderer to schedule revalidation. The
* public phase functions are for user work; revalidation is hotter
* than any user phase, so the renderer talks to the strategy through
* this callback-based hook rather than allocating promises per
* invalidation. Optional: strategies that do not implement it leave
* the renderer on its classic runloop scheduling.
*
* @internal
*/
_scheduleRevalidate?(flush: () => void): void;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is this here? we can't add to the interface of the scheduler, even for compat

}

let registeredStrategy: Strategy | null = null;
/**
* The active strategy, as a live binding for the renderer's hot path.
* The framework registers the classic strategy at the glimmer<->ember
* hookup during boot, so this is non-null in any booted application.
*
* @internal
*/
export let _registeredStrategy: Strategy | null = null;

/**
Registers the scheduling strategy which the phase functions of
Expand Down Expand Up @@ -113,22 +133,16 @@ let registeredStrategy: Strategy | null = null;
export function registerStrategy(strategy: Strategy): void {
assert(
'Cannot call `registerStrategy`: a different scheduling strategy has already been registered. The scheduling strategy should be registered exactly once, when defining the Application.',
registeredStrategy === null || registeredStrategy === strategy
_registeredStrategy === null ||
_registeredStrategy === strategy ||
_registeredStrategy === classicStrategy

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this third comparison is unneeded, and would be handled by the comparison above

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to test this proper, you'd want a well known symbol to check on the internal strategies, because we do want those to be overwritten

);
registeredStrategy = strategy;
_registeredStrategy = strategy;
}

// Private API used by tests to swap out the registered strategy.
export function _clearRegisteredStrategy(): void {
registeredStrategy = null;
}

function getStrategy(phaseName: string): Strategy {
assert(
`Attempted to schedule work into the '${phaseName}' phase, but no scheduling strategy is registered. Register a strategy when defining your Application, e.g. the default strategy:\n\n\timport { registerStrategy } from '@ember/scheduler';\n\timport strategy from '@ember/scheduler/strategy';\n\n\tregisterStrategy(strategy);`,
registeredStrategy !== null
);
return registeredStrategy;
_registeredStrategy = null;
}

/**
Expand Down Expand Up @@ -156,7 +170,11 @@ function getStrategy(phaseName: string): Strategy {
@public
*/
export function render(): Promise<void> {
return getStrategy('render').render();
assert(
`Attempted to schedule work into the 'render' phase before a scheduling strategy was available. The framework registers the default strategy during boot.`,
_registeredStrategy !== null
);
return _registeredStrategy.render();
}

/**
Expand All @@ -182,7 +200,11 @@ export function render(): Promise<void> {
@public
*/
export function layout(): Promise<void> {
return getStrategy('layout').layout();
assert(
`Attempted to schedule work into the 'layout' phase before a scheduling strategy was available. The framework registers the default strategy during boot.`,
_registeredStrategy !== null
);
return _registeredStrategy.layout();
}

/**
Expand Down Expand Up @@ -212,7 +234,11 @@ export function layout(): Promise<void> {
@public
*/
export function composite(): Promise<void> {
return getStrategy('composite').composite();
assert(
`Attempted to schedule work into the 'composite' phase before a scheduling strategy was available. The framework registers the default strategy during boot.`,
_registeredStrategy !== null
);
return _registeredStrategy.composite();
}

/**
Expand All @@ -237,7 +263,11 @@ export function composite(): Promise<void> {
@public
*/
export function next(): Promise<void> {
return getStrategy('next').next();
assert(
`Attempted to schedule work into the 'next' phase before a scheduling strategy was available. The framework registers the default strategy during boot.`,
_registeredStrategy !== null
);
return _registeredStrategy.next();
}

/**
Expand All @@ -261,5 +291,9 @@ export function next(): Promise<void> {
@public
*/
export function idle(): Promise<void> {
return getStrategy('idle').idle();
assert(
`Attempted to schedule work into the 'idle' phase before a scheduling strategy was available. The framework registers the default strategy during boot.`,
_registeredStrategy !== null
);
return _registeredStrategy.idle();
}
5 changes: 3 additions & 2 deletions packages/@ember/scheduler/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
},
"dependencies": {
"@ember/debug": "workspace:*",
"internal-test-helpers": "workspace:*"
"internal-test-helpers": "workspace:*",
"@ember/runloop": "workspace:*"
}
}
}
61 changes: 59 additions & 2 deletions packages/@ember/scheduler/tests/scheduler_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@ import {
idle,
registerStrategy,
_clearRegisteredStrategy,
_registeredStrategy,
} from '..';
import classicStrategy from '../-private/classic';
import { moduleFor, AbstractTestCase } from 'internal-test-helpers';

class StubStrategy {
Expand Down Expand Up @@ -45,16 +47,57 @@ moduleFor(
_clearRegisteredStrategy();
}

['@test phase functions assert when no strategy is registered'](assert) {
['@test phase functions assert before any strategy is registered'](assert) {
// the framework hookup registered classic when the bundle loaded;
// simulate the pre-boot state
_clearRegisteredStrategy();

for (let phase of [render, layout, composite, next, idle]) {
expectAssertion(() => {
phase();
}, /no scheduling strategy is registered/);
}, /before a scheduling strategy was available/);
}

assert.expect(5);
}

async ['@test the classic strategy resolves phases in runloop order'](assert) {
// the framework registers this at the glimmer<->ember hookup
// during boot; tests clear registration, so re-register here
registerStrategy(classicStrategy);

let order = [];

await Promise.all([
composite().then(() => order.push('composite')),
layout().then(() => order.push('layout')),
render().then(() => order.push('render')),
]);

assert.deepEqual(order, ['render', 'layout', 'composite']);
}

['@test the renderer seam prefers a registered strategy that implements it'](assert) {
let scheduled = [];

registerStrategy({
render: () => Promise.resolve(),
layout: () => Promise.resolve(),
composite: () => Promise.resolve(),
next: () => Promise.resolve(),
idle: () => Promise.resolve(),
_scheduleRevalidate(flush) {
scheduled.push(flush);
},
});

let flush = () => {};
_registeredStrategy._scheduleRevalidate(flush);

assert.strictEqual(scheduled.length, 1, 'the registered strategy received the flush');
assert.strictEqual(scheduled[0], flush, 'with the stable callback');
}

['@test phase functions delegate to the registered strategy'](assert) {
let strategy = new StubStrategy();
registerStrategy(strategy);
Expand Down Expand Up @@ -84,6 +127,20 @@ moduleFor(
}
}

['@test registerStrategy may replace the classic default, once'](assert) {
registerStrategy(classicStrategy);

let strategy = new StubStrategy();
registerStrategy(strategy);

render();
assert.deepEqual(strategy.calls, ['render'], 'the swapped-in strategy is active');

expectAssertion(() => {
registerStrategy(new StubStrategy());
}, /a different scheduling strategy has already been registered/);
}

['@test registerStrategy asserts when a different strategy is already registered'](assert) {
let strategy = new StubStrategy();
registerStrategy(strategy);
Expand Down
Loading