profiler icon indicating copy to clipboard operation
profiler copied to clipboard

Make inverting the call tree fast, by computing inverted call nodes lazily

Open mstange opened this issue 1 year ago • 2 comments

Deploy preview

Fixes #467, fixes #337, fixes #3313.

Some changes that could still be made to this PR:

  • [x] A commit at the end which removes bisectLowerBound(), bisectUpperBound(), and bisectEqualRange() again
  • [ ] More comments around the lazy inverted call node info:
    • [x] How the suffix order is computed incrementally as call nodes are created
    • [x] Something about "deep nodes", i.e. the n'th parent of a non-inverted node which corresponds to an inverted node at depth n
    • [ ] More about how children are created, and that a node either knows about all of its children or none of them; in other words, if a node has been created, all its sibling nodes have been created, too
  • [ ] A test which checks inverted diff profiles (call nodes with 0 diff time should be visible if they have non-zero diff descendants)
  • [x] Find out why code coverage is saying that some methods CallNodeInfoInvertedImpl are never being hit, and possibly add a test or remove code

┆Issue is synchronized with this Jira Task

mstange avatar Jan 23 '24 16:01 mstange

Codecov Report

Attention: Patch coverage is 94.41261% with 39 lines in your changes missing coverage. Please review.

Project coverage is 86.04%. Comparing base (39c4722) to head (0cd70fa).

Files with missing lines Patch % Lines
src/profile-logic/call-node-info.js 93.86% 21 Missing and 2 partials :warning:
src/profile-logic/profile-data.js 92.03% 9 Missing :warning:
src/profile-logic/call-tree.js 96.75% 5 Missing :warning:
src/components/flame-graph/FlameGraph.js 66.66% 1 Missing :warning:
src/components/stack-chart/index.js 0.00% 1 Missing :warning:
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4900      +/-   ##
==========================================
+ Coverage   85.93%   86.04%   +0.11%     
==========================================
  Files         312      312              
  Lines       29773    30271     +498     
  Branches     8197     8275      +78     
==========================================
+ Hits        25585    26047     +462     
- Misses       3597     3631      +34     
- Partials      591      593       +2     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Jan 23 '24 20:01 codecov[bot]

This is now ready for review! A few more comments are probably needed in the "Create inverted call nodes lazily" commit, but I think it's reviewable in its current state.

mstange avatar Aug 07 '24 23:08 mstange

I've now split the incremental order refinement out into a separate commit at the end. I've also polished both split commits to be easier to read.

Both the "Create inverted call nodes lazily" and the "Refine the suffix order incrementally, as new inverted nodes are created" commit should be much easier to review now.

mstange avatar Nov 24 '24 19:11 mstange