Skip to content

feat: attach CPU timing metadata to render event#7768

Closed
akre54 wants to merge 3 commits into
maplibre:mainfrom
akre54:feat/render-timing-metadata
Closed

feat: attach CPU timing metadata to render event#7768
akre54 wants to merge 3 commits into
maplibre:mainfrom
akre54:feat/render-timing-metadata

Conversation

@akre54

@akre54 akre54 commented Jun 10, 2026

Copy link
Copy Markdown

Summary

Adds CPU timing metadata to the render event for frame performance monitoring and capture timing decisions.

Motivation

The render event fires after each paint but provides no timing information. For video export and performance analysis (e.g., noodles.gl), knowing render duration helps:

  1. Distinguish trivial updates (style changes) from substantial renders (tile painting)
  2. Detect performance bottlenecks
  3. Estimate GPU buffer swap timing

API

```typescript
map.on('render', (event: MapLibreRenderEvent) => {
// New timing property
event.timing: {
start: number, // performance.now() when render started
end: number, // performance.now() after painter.render()
duration: number // end - start (ms)
}
})
```

All timestamps are DOMHighResTimeStamp (milliseconds since page navigation).

Note: These are CPU-side timestamps. WebGL exposes no portable signal for GPU buffer-swap completion. For true frame readiness, also listen for viewporttilesloaded (#7767) or use canvas.captureStream(0).

Implementation

  • Brackets painter.render() with performance.now() calls
  • Adds MapLibreRenderTiming and MapLibreRenderEvent types (exported from public API)
  • Timing field always populated (stable event shape for TypeScript consumers)
  • Cost: two performance.now() calls per frame (~0.0003% of 16.67ms frame time)

Tests

1 new test verifying:

  • Timing property exists
  • end ≥ start
  • duration matches bracket
  • Reasonable values for synthetic test

All 1163 UI tests pass.

Usage Example

```typescript
map.on('render', (event) => {
const { start, end, duration } = event.timing;
console.log(`Render: ${duration.toFixed(1)}ms`);

// Detect substantial renders
if (duration > 10) {
console.log('Heavy render - likely tile painting');
}
});
```

Performance

~47ns overhead per frame (two performance.now() calls) — 0.0003% of 16.67ms frame budget.

Breaking Changes

None. MapEventType.render type narrowed from MapLibreEvent to MapLibreRenderEvent (structural superset, backward compatible).

Related PRs

Companion deck.gl PRs


Co-authored-by: Claude Sonnet 4.5 noreply@anthropic.com

Attaches a `timing` property to the `render` event payload with
high-resolution `performance.now()` timestamps so consumers (capture
pipelines, profilers, performance dashboards) can reason about render
cost without instrumenting maplibre internals.

The new payload shape (see MapLibreRenderTiming):
- renderStart:       performance.now() at painter.render() entry
- commandsSubmitted: performance.now() when painter.render() returns
                     (i.e. when WebGL commands have been submitted)
- renderDuration:    commandsSubmitted - renderStart, in ms

WebGL exposes no portable buffer-swap signal, so we deliberately do not
fabricate a "frame on screen" timestamp — the doc-comment on the type
points capture/export consumers at viewporttilesloaded and at compositor
APIs (canvas.captureStream) for that.

Type changes:
- New MapLibreRenderTiming type
- New MapLibreRenderEvent = MapLibreEvent & {timing: MapLibreRenderTiming}
- MapEventType.render is now MapLibreRenderEvent (was MapLibreEvent).
  This is a structural superset, so existing listeners typed as
  MapLibreEvent continue to compile.

Implementation cost is two performance.now() calls per frame, so the
field is populated unconditionally; this keeps the event shape stable
for typed consumers and avoids "is timing here?" branching at call sites.

Tested via map_render.test.ts: verifies the property exists, the
ordering invariant (commandsSubmitted >= renderStart), and that
renderDuration matches the bracket within fp tolerance.
@codecov

codecov Bot commented Jun 10, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.12%. Comparing base (06ee5a2) to head (b26984c).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #7768   +/-   ##
=======================================
  Coverage   93.12%   93.12%           
=======================================
  Files         288      288           
  Lines       24427    24429    +2     
  Branches     6452     6452           
=======================================
+ Hits        22748    22750    +2     
  Misses       1679     1679           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

…ic API

Both new types were defined and `export type`d from `src/ui/events.ts`,
but the public API barrel `src/index.ts` did not re-export them, so:

- TypeScript users couldn't import them by name (e.g.
  `import type { MapLibreRenderEvent } from 'maplibre-gl'`) when
  annotating their `render` listeners.
- typedoc emitted a referenced-but-not-included warning, which fails
  the docs build because typedoc.json sets `treatWarningsAsErrors: true`.

Add both type names to the existing event-related re-exports alongside
`MapLibreEvent`. Verified with `npm run generate-docs` (no warnings)
and `npm run typecheck` (clean).
@akre54

akre54 commented Jun 10, 2026

Copy link
Copy Markdown
Author

Pushed a follow-up commit (049e3d4) that re-exports MapLibreRenderEvent and MapLibreRenderTiming from src/index.ts.

The types were defined and export typed from src/ui/events.ts in the original commit, but I missed adding them to the public API barrel. As a result:

  • TypeScript users couldn't import type { MapLibreRenderEvent } from 'maplibre-gl' when annotating their render listeners.
  • typedoc emitted a "referenced but not included" warning, which fails the docs build (typedoc.json has treatWarningsAsErrors: true).

Verified locally: npm run generate-docs is now warning-free and produces docs/API/type-aliases/MapLibreRenderEvent.md + MapLibreRenderTiming.md. npm run typecheck is also clean.

Comment thread src/ui/map.ts Outdated
this._placementDirty = this.style?._updatePlacement(this.transform, this.showCollisionBoxes, fadeDuration, this._crossSourceCollisions, globeRenderingChanged);

// Actually draw
// Actually draw. Bracket painter.render() with high-resolution timestamps

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think this comment is a bit too verbose... Consider removing it.

Comment thread src/ui/map.ts Outdated
// so we can attach CPU-side timing metadata to the `render` event below.
// The cost is two performance.now() calls per frame; populating the field
// unconditionally keeps the event shape stable for typed consumers.
const renderStart = performance.now();

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Why was this added here and not at the begging of this method?

Comment thread src/ui/events.ts
*
* @group Event Related
*/
export type MapLibreRenderTiming = {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

As you said, I think start/end/duration is better

Comment thread src/ui/map_tests/map_render.test.ts Outdated
const event = await renderPromise;

expect(event.timing).toBeDefined();
expect(typeof event.timing.renderStart).toBe('number');

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I dont think these type e check are interesting...

Comment thread src/ui/map_tests/map_render.test.ts Outdated
// Sanity: end is at or after start, duration matches the bracket.
expect(event.timing.commandsSubmitted).toBeGreaterThanOrEqual(event.timing.renderStart);
expect(event.timing.renderDuration).toBeCloseTo(event.timing.commandsSubmitted - event.timing.renderStart, 5);
// Duration should be non-negative and reasonable for a synthetic test (well under a second).

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Remove comments...

@HarelM

HarelM commented Jun 10, 2026

Copy link
Copy Markdown
Collaborator

Thanks for this PR! Added mostly superficial comments.

@akre54

akre54 commented Jun 10, 2026

Copy link
Copy Markdown
Author

Done! Thanks for the quick review

@akre54 akre54 marked this pull request as ready for review June 10, 2026 06:07
@HarelM

HarelM commented Jun 10, 2026

Copy link
Copy Markdown
Collaborator

Not that I think the code added here is problematic, but I still need to ask, can't you just calculate this by listening to the render event right now?

@akre54

akre54 commented Jun 10, 2026

Copy link
Copy Markdown
Author

Good question! The issue is that the render event fires after painter.render() completes, so by the time your listener receives it, you've already missed the start time.

Users would need to hook before the render starts and after it completes. Without this PR, the options are:

  1. Monkey-patch _render() - fragile, breaks on internal changes
  2. Wrap the Map instance - same issues, plus TypeScript complications
  3. Use two separate events - no existing "beforeRender" event

This PR brackets painter.render() from inside _render() where we have access to both the start and end points.

Alternative we considered: Add separate beforeRender and afterRender events, but:

  • That's two events per frame instead of enriching the existing render event
  • Users would need to correlate pairs of events (race conditions, dropped events)
  • The timing field keeps the render event self-contained

The cost is minimal (two performance.now() calls) and the timing data is useful beyond just video export - performance monitoring, frame budget tracking, etc.

Does that address the concern, or would you prefer a different approach?

@akre54

akre54 commented Jun 10, 2026

Copy link
Copy Markdown
Author

The AI got a little overeager here (I didn't ask for it to post as me). But mostly I'm just looking for ways of speeding up Noodles rendering: joby-aviation/noodles.gl#452

@HarelM

HarelM commented Jun 10, 2026

Copy link
Copy Markdown
Collaborator

But aren't render events fired one after the other? Can't you measure "time since last render event"? Would that give different results?

@akre54

akre54 commented Jun 10, 2026

Copy link
Copy Markdown
Author

They're slightly different.

The interval includes:

  • Previous render work
  • Idle time (waiting for RAF, other JS execution)
  • Current render work

For video export, we care about how long rendering took (is this a heavy tile paint vs a quick style update?), not how long since the last frame.

The interval timing would give framerate data, not render performance data. Different use case.


expect(event.timing).toBeDefined();
expect(event.timing.end).toBeGreaterThanOrEqual(event.timing.start);
expect(event.timing.duration).toBeCloseTo(event.timing.end - event.timing.start, 5);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Will this check survive CI flackiness?

Comment thread src/ui/events.ts
*
* @group Event Related
*/
export type MapLibreRenderEvent = MapLibreEvent & {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Man.. this is a mess, I now see that some events are classes and some are types, I'm not sure I even know why...
My original thought was that this event should have the timing as it's properties instead of a nested obejct, but then I got confused with the different event implementations... :-/
I don't know which is better to be honest...

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

What's the benefit of a class over a nested object?

@CommanderStorm CommanderStorm Jun 10, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

you can instanceof and can inherit.

@CommanderStorm

CommanderStorm commented Jun 10, 2026

Copy link
Copy Markdown
Member

For video export, we care about how long rendering took (is this a heavy tile paint vs a quick style update?), not how long since the last frame.

The interval timing would give framerate data, not render performance data. Different use case.

Really stupid question, but what is difference in your context and why do you care about that difference?

You know that you can control our render timing (so what time in the animation we render at) That API is relatively new..
What you are trying to do sounds suspiciously similar.

@akre54

akre54 commented Jun 10, 2026

Copy link
Copy Markdown
Author

This PR to me is more about general instrumentation, less about the video use case specifically. If it's at all controversial I'm fine dropping

@CommanderStorm

Copy link
Copy Markdown
Member

I would be fine with including that part of the timing data if I were to know why you would want this.
Frame timing can be taken by listening on render and measuring the time in betweeen...

https://maplibre.org/maplibre-gl-js/docs/examples/display-performance-metrics/

@akre54

akre54 commented Jun 12, 2026

Copy link
Copy Markdown
Author

@HarelM @CommanderStorm valid feedback. Closing this PR. Thanks for the review.

@akre54 akre54 closed this Jun 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants