1.32 KB, text/html
1.92 KB, patch
|Details | Diff | Splinter Review|
46 bytes, text/x-phabricator-request
|Details | Review|
(Spinning this off for part of bug 1377253 that I want to land on its own.) Right now nsFlexContainerFrame::MarkIntrinsicISizesDirty clears the CachedFlexMeasuringReflow() frame-property on all flex items. This is unnecessary in most cases. I think the flex items' CachedFlexMeasuringReflow are still valid most of the time, though I need to think harder about whether there are scenarios where we do need to discard the CachedFlexMeasuringReflow data. (One case where we definitely need to discard it: when the item itself has its intrinsic sizes marked as dirty. We can handle that in the generic nsFrame::MarkIntrinsicISizesDirty method, similar to how we invalidate for XUL boxes there.)
Here's a testcase, based on the codemirror scenario from bug 1485084. This testcase logs two measurements. * For the first one ("Layout duration for append:"), we happen to be faster than Chrome right now. * For the second one, "Layout duration for tweak in other flex item"), we currently report ~260ms, whereas Chrome reports 0ms or 1ms. This bug is about getting us to be closer to the 0ms / 1ms range for this second measurement.
Attachment #9008611 - Attachment description: testcase 1 → testcase 1 (benchmark which logs to console)
(In reply to Daniel Holbert [:dholbert] from comment #1) > Here's a testcase, based on the codemirror scenario from bug 1485084. Specifically: the second measurement in this testcase (the measurement I'm focusing on improving here) is basically measuring the keypress reaction time in the CodeMirror "typing in web console" scenario.
Here's what I've got at the moment, though I need to convince myself that it's correct (and maybe add some special cases if it's not). This is a subset of the WIP patch that I'd posted for feedback in bug 1485084 comment 5, btw. I'm pretty sure this is the key part of that patch that helps for this CodeMirror / web-console-interactivity use case.
Assignee: nobody → dholbert
The "WIP" patch drops testcase 1's 2nd measurement (the one I'm focusing on here) to the 5-6ms range, rather than ~260ms+.
(cloning qf status from bug 1377253 which this is a helper for)
Right now, when a flex item's intrinsic size is invalidated, we clear cached flex measurements on all of its sibling flex items (indirectly, by virtue of invalidating the flex container's intrinsic sizes, which does the dirty work of clearing the measurements). This is excessive. We do need to clear the measurements on any flex item whose intrinsic sizes are invalidated, but we don't need to clear them on other flex items whose intrinsic sizes are still valid. So: this patch changes our implementation accordingly. This patch isn't expected to change behavior - it should just be an optimization.
Comment on attachment 9009287 [details] Bug 1490890: Make flex item cached-measurement invalidation more targeted. r?emilio Emilio Cobos Álvarez (:emilio) has approved the revision.
Attachment #9009287 - Flags: review+
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/autoland/rev/7845d51b6f3c Make flex item cached-measurement invalidation more targeted. r=emilio
Status: NEW → RESOLVED
Last Resolved: 6 months ago
status-firefox64: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
You need to log in before you can comment on or make changes to this bug.