Closed
Bug 1451021
Opened 6 years ago
Closed 6 years ago
Convert PropertyIteratorActor to protocol.js
Categories
(DevTools :: General, enhancement)
DevTools
General
Tracking
(firefox61 fixed)
RESOLVED
FIXED
Firefox 61
Tracking | Status | |
---|---|---|
firefox61 | --- | fixed |
People
(Reporter: yulia, Assigned: nchevobbe)
References
Details
Attachments
(1 file)
As part of refactoring ObjectActor to protocol.js, we need to update PropertyIteratorActor. it is instantiated via: -> ObjectActor.enumProperties/enumEntries
Assignee | ||
Updated•6 years ago
|
Assignee: nobody → nchevobbe
Status: NEW → ASSIGNED
Assignee | ||
Comment 1•6 years ago
|
||
I have some test failures I need to check (one unrelated failure in inspector, another one in old debugger), but I pushed to DAMP to see the impact: https://treeherder.mozilla.org/perf.html#/compare?originalProject=try&originalRevision=80c7d4f34a94c15aaa467f6ec0896f29734344c5&newProject=try&newRevision=16e4dc085e1dd2082a149efee0f2b774414ea438&framework=1 objectexpand seems to be around ~1% slower.
Comment 2•6 years ago
|
||
(In reply to Nicolas Chevobbe [:nchevobbe] from comment #1) > I have some test failures I need to check (one unrelated failure in > inspector, another one in old debugger), but I pushed to DAMP to see the > impact: > https://treeherder.mozilla.org/perf.html#/ > compare?originalProject=try&originalRevision=80c7d4f34a94c15aaa467f6ec0896f29 > 734344c5&newProject=try&newRevision=16e4dc085e1dd2082a149efee0f2b774414ea438& > framework=1 > > objectexpand seems to be around ~1% slower. FWIW I don't think a 1% regression on that single subtest should prevent landing something here. If it's due to some inherent slowness to protocol.js then fixing that (in separate bugs) should be a win across the board.
Comment hidden (mozreview-request) |
Assignee | ||
Comment 4•6 years ago
|
||
TRY is fine https://treeherder.mozilla.org/#/jobs?repo=try&revision=c70959a95336be03265bf3839d002fdfda708be4
Comment 5•6 years ago
|
||
mozreview-review |
Comment on attachment 8968603 [details] Bug 1451021 - Migrate PropertyIteratorActor to protocol.js; . https://reviewboard.mozilla.org/r/237296/#review243268 Thanks Nicolas, it looks good. It sounds fine to proceed with such a small regression, especially given that I still have various pending patches that may revert it. But note that if you have another refactoring introducing significant regression, it is the best time to look into it as looking at perf-html profiles of the regressing DAMP subtest often highlight the culprit within protocol.js
Attachment #8968603 -
Flags: review?(poirot.alex) → review+
Pushed by nchevobbe@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/97a6ec0981d0 Migrate PropertyIteratorActor to protocol.js; r=ochameau.
Comment 7•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/97a6ec0981d0
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
status-firefox61:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 61
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•