Closed Bug 1451021 Opened 2 years ago Closed 2 years ago
Iterator Actor to protocol .js
59 bytes, text/x-review-board-request
As part of refactoring ObjectActor to protocol.js, we need to update PropertyIteratorActor. it is instantiated via: -> ObjectActor.enumProperties/enumEntries
Assignee: nobody → nchevobbe
Status: NEW → ASSIGNED
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.
(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 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 email@example.com: https://hg.mozilla.org/integration/autoland/rev/97a6ec0981d0 Migrate PropertyIteratorActor to protocol.js; r=ochameau.
You need to log in before you can comment on or make changes to this bug.