Closed
Bug 1452936
Opened 6 years ago
Closed 6 years ago
Add symbols to the object being expanded in DAMP objectexpand.js
Categories
(DevTools :: Console, defect)
DevTools
Console
Tracking
(firefox61 fixed)
RESOLVED
FIXED
Firefox 61
Tracking | Status | |
---|---|---|
firefox61 | --- | fixed |
People
(Reporter: nchevobbe, Assigned: nchevobbe)
References
Details
Attachments
(1 file)
The current test expands an object with simple properties [1], which means we can miss regression on object with symbols. [1] https://searchfox.org/mozilla-central/rev/b55e1a1cbcaee34878e133fbac20c4c2af6e11b5/testing/talos/talos/tests/devtools/addon/content/tests/webconsole/objectexpand.js#30-37
Comment hidden (mozreview-request) |
Assignee | ||
Updated•6 years ago
|
Assignee: nobody → nchevobbe
Status: NEW → ASSIGNED
Assignee | ||
Comment 2•6 years ago
|
||
I pushed to DAMP to see if there is any impact https://treeherder.mozilla.org/perf.html#/compare?originalProject=try&originalRevision=70da3e5b12a955cca25a389fb73fdb43963df6ce&newProject=try&newRevision=3ac16d3bb5ac8e6faf9301a3ace6e98a8a59c32c&framework=1 The number of properties on the object stay ~ the same (was 1000, now 999), and we should have a better view of what it takes to fetch different type of properties on an object.
Assignee | ||
Comment 3•6 years ago
|
||
DAMP shows a regression in the test (actual expanding - ~4% - and closing - ~7%). I guess the overhead is due to Symbols indexes being more complex (we have to go through Symbol rep). Alex, does it looks reasonable to you ?
Comment 4•6 years ago
|
||
mozreview-review |
Comment on attachment 8966570 [details] Bug 1452936 - Modify DAMP objectexpand.js to cover more property types; . https://reviewboard.mozilla.org/r/235294/#review241294 Thanks for this tweak. (In reply to Nicolas Chevobbe [:nchevobbe] from comment #3) > DAMP shows a regression in the test (actual expanding - ~4% - and closing - > ~7%). > I guess the overhead is due to Symbols indexes being more complex (we have > to go through Symbol rep). > Alex, does it looks reasonable to you ? Yes, I will flag that as a test change increase.
Attachment #8966570 -
Flags: review?(poirot.alex) → review+
Pushed by nchevobbe@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/7c6d97a76b96 Modify DAMP objectexpand.js to cover more property types; r=ochameau.
Comment 6•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/7c6d97a76b96
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
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
•