Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feedback on the proposal #36

Open
jasnell opened this issue Jan 18, 2021 · 1 comment
Open

Feedback on the proposal #36

jasnell opened this issue Jan 18, 2021 · 1 comment

Comments

@jasnell
Copy link

jasnell commented Jan 18, 2021

First off, a thank you @littledan for pointing me at this proposal. Overall I think it's a great start and a great addition to the API surface here. It should be fairly straightforward to implement support for this in Node.js and definitely think it's something we would do.

A couple of comments on the API... May have more later but these are what jumps out immediately.

  1. An alternative to specifying a sample interval would be to specify a sample rate ... for instance, something like await performance.profile({ hz: 20 }) (sample at approximately 20 samples per second).

  2. The ProfilerTrace object can potentially be a bit heavy in terms of memory load. It would be very interesting to support a streaming mode as an alternative that just outputs a raw stream of profile data rather than building up the index. Imagine an API like...

const profiler = await performance.profile({ sampleInterval: 10 });
for (let i = 0; i < 1000000; i++) {
     doWork();
}
profiler.stop();

sendTrace(await profiler.trace());  // returns the parsed `ProfilerTrace`

// alternatively

const profiler = await performance.profile({ sampleInterval: 10 });
profiler.reader().pipeTo(writable);  // Consumes the profile as a raw stream of events

for (let i = 0; i < 1000000; i++) {
     doWork();
}

profiler.stop();

The stream approach would generate more raw unprocessed data but would allow it to be piped out and processed later much more efficiently.

@nicjansma
Copy link

For (2), a stream is interesting because it could provide a workaround for cases where the Profiler is active and the page is unloading. In that case, the website may wish to stop-and-send the profile, but the Promise from Profiler.stop() may prevent that from happening (since the page will move-on with unloading before the Promise resolves).

If there was a stream available, the website code could maintain the profile stream on its own, and be able to beacon the profile at unload without waiting for a Promise.

I think the challenge is that there are multiple parts to "stream" though, looking at the current output of a Profiler.stop(), where you get frames, resources, samples and stacks top-level objects. Each of those would need to be "streamed" in a way, or the stream would need to be notified if there were any new entries in any of the above.

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

No branches or pull requests

2 participants