1452936 - Add symbols to the object being expanded in DAMP objectexpand.js

Status: RESOLVED FIXED

(DevTools :: Console, defect)

Description

[Nicolas Chevobbe [:nchevobbe]]

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 1

[Nicolas Chevobbe [:nchevobbe]]

Attachment: Bug 1452936 - Modify DAMP objectexpand.js to cover more property types; .

The test was measuring the time spent to expand an object only
containing alphanumeric properties. But due to how we fetch properties,
this only cover a part of data fetching performances.
This patch adds numeric and Symbol indexes in order to have a more precise
view of what is happening when expanding an object in the console.

Review commit: https://reviewboard.mozilla.org/r/235294/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/235294/

Comment 2

[Nicolas Chevobbe [:nchevobbe]]

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.

Comment 3

[Nicolas Chevobbe [:nchevobbe]]

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

[Alexandre Poirot [:ochameau]]

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.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7c6d97a76b96
Modify DAMP objectexpand.js to cover more property types; r=ochameau.

Comment 6

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/7c6d97a76b96