Add Gecko's phased markers to the front-end #2626
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report
@@ Coverage Diff @@
## main #2626 +/- ##
==========================================
- Coverage 86.59% 86.54% -0.06%
==========================================
Files 219 218 -1
Lines 17437 17638 +201
Branches 4529 4542 +13
==========================================
+ Hits 15100 15264 +164
- Misses 2141 2171 +30
- Partials 196 203 +7
Continue to review full report at Codecov.
|
julienw
reviewed
Jul 3, 2020
This was referenced Jul 7, 2020
gregtatum
force-pushed
the
raw-marker-start-end
branch
2 times, most recently
from
fb84915
to
f8b69e0
Compare
This was referenced Jul 20, 2020
gregtatum
changed the title
… (partial) This patch does not fix tests, nor satisfy the Flow checks. It is only the work to modify the RawMarkerTable to add the new phased marker support. Imports, tests, fixing the marker pipeline, and passing flow are only for future patches.
I'm not completely sure why this happened, but it didn't seem like a regression.
… markers, previously they were exclusive
These snapshots add the phase, but they also change the fetchStart property of the network markers. This is due to changes in the underlying fixture generation.
…AL_START markers This isn't particularly useful, as the marker buckets aren't REALLY truncating long markers, just relying on the interval mechanism of deriving markers. This is already covered elsewhere.
Ok, so this lump of snapshot changes are a bit mysterious, but I think that's partly due to limitations in snapshot testing not being specific. I'm somewhat curious if these will be hiding regressions when I load this up with real profile data.
The only places this was left in was with Network and IPC payloads, as there was logic using these values. It was much easier to leave them in for now.
This will finally teach our Markers the difference between Instant and Interval markers.
…ction for deriving markers
…s more verbose in naming
gregtatum
force-pushed
the
raw-marker-start-end
branch
from
f8b69e0
to
7d9bead
Compare
canova
approved these changes
Jul 24, 2020
… instead of marker.start
…de into the upgraders The upgraders were sharing code with the type system, which the startTime and endTime changes ended up being a breaking change. In order to properly isolate this code, it was duplicated and inlined in the upgraders.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is the unified patch to handle the Gecko patch from: https://phabricator.services.mozilla.com/D78990
This PR will serve as the actual merge point for this work, but I've broken it out into smaller, more reviewable pieces:
Review status:
startTimeandendTimefrom payloads Remove startTime/endTime from most marker payloads (Do not merge, see #2626) #2662Markertype'sdurproperty toendTime(follow-up?) Change the Marker type to use end rather dur (Do not merge, see #2626) #2660