Bug 1895393 - Populate the FireScrollEvent profiler marker with a stack trace of where scrolling was triggered r?botond 1 month ago Vignesh B [:vsr-b] 48 bytes, text/plain	Details
Bug 1895393 - Populate the FireScrollEvent profiler marker with a stack trace of where scrolling was triggered r?botond 1 month ago Vignesh B [:vsr-b] 48 bytes, text/x-phabricator-request	Details \| Review

👁 Image	Botond Ballo [:botond] [away until Jun 24] Reporter
	Description • 2 years ago

nsHTMLScrollFrame::FireScrollEvent creates a profiler marker here.

For better understanding the cause of scrolling seen in a profile, it would be helpful if this marker contained a stack trace showing where scrolling was triggered.

This would not be a stack trace for the FireScrollEvent call itself (which happens asynchronously with respect to the actual scrolling, on the next refresh driver tick), but a stack trace of the PostScrollEvent call that queued the scroll event that is fired.

Based on discussion with Markus, we should be able to collect the stack trace in PostScrollEvent and stash it for later attaching to the marker in FireScrollEvent.

An alternative approach could be to have PostScrollEvent produce its own profiler marker with a stack trace.

👁 Image	Hiroyuki Ikezoe (:hiro)
	Comment 1 • 2 years ago

It would be nice to differentiate scroll anchoring.

👁 Image

Botond Ballo [:botond] [away until Jun 24]

Reporter

Updated

•

3 months ago

Mentor: botond

Keywords: good-next-bug

Whiteboard: [lang=c++]

👁 Image

Botond Ballo [:botond] [away until Jun 24]

Reporter

Updated

•

2 months ago

See Also: → 2026854

👁 Image	Vignesh B [:vsr-b] Assignee
	Comment 2 • 1 month ago

Hi :botond, I'd like to work on this bug. The expected is to capture a stack trace in PostScrollEvent using profiler_capture_backtrace(), stash on scroll and attach using MarkerStack::TakeBacktrace() when a profiler is created in FireScrollEvent. Hope I'm in right direction.

Flags: needinfo?(botond)

👁 Image	Botond Ballo [:botond] [away until Jun 24] Reporter
	Comment 3 • 1 month ago

(In reply to B Vignesh Sai Raghav [:vignesh-b] from comment #2)

Hi :botond, I'd like to work on this bug.

Hi, thanks for your interest!

The expected is to capture a stack trace in PostScrollEvent using profiler_capture_backtrace(), stash on scroll and attach using MarkerStack::TakeBacktrace() when a profiler is created in FireScrollEvent. Hope I'm in right direction.

Yep, I believe that's on the right track.

One detail to figure out is how to pass the backtrace into the AUTO_PROFILER_MARKER_DOCSHELL macro.

Looking at what this macro does, it constructs an object of type AutoProfilerTracing, currently using this constructor.

The constructor in question does not take a backtrace, but this one does.

So my suggestion would be to revise the AUTO_PROFILER_MARKER_DOCSHELL macro to take an additional parameter for the backtrace, and have the macro implementation forward that parameter to the AutoProfilerTracing constructor. There is no need to call MarkerStack::TakeBacktrace() directly, that's done by the constructor.

Let me know if that makes sense; I'm happy to answer any follow-up questions.

Flags: needinfo?(botond)

👁 Image	Vignesh B [:vsr-b] Assignee
	Comment 4 • 1 month ago

Thank you for the very detailed explanation! That makes a lot of sense!

The final approach -

PostScrollEvent(), capture the backtrace using profiler_capture_backtrace() and stash to ScrollEvent object.
FireScrollEvent(), retrieve the backtrace from mScrollEvent.
Modify AUTO_PROFILER_TRACING_MARKER_DOCSHELL to accept an additional backtrace parameter and forward it to the AutoProfilerTracing .
Pass the retrieved backtrace when calling FireScrollEvent().

Happy to send a patch for review!

Flags: needinfo?(botond)