Open Bug 1387823 Opened 7 years ago Updated 2 years ago

Object inspector freezes with huge typed arrays

Categories

(DevTools :: Console, defect, P2)

Product:
DevTools
Component:
Console
Version:
56 Branch
Type:
defect

Tracking

(firefox56 unaffected, firefox57 fix-optional)

Tracking Flags:
Tracking Status
firefox56 --- unaffected
firefox57 --- fix-optional

People

(Reporter: Oriol, Unassigned)

References

(Depends on 1 open bug, Blocks 2 open bugs)

Details

1. Open web console
2. Enter this code:
   new Uint8Array(1e5)
3. Click the resulting object.

Since it's a huge object, with variablesView you see slices:

   ▶ [0...24999]
   ▶ [25000...44999]
   ▶ [50000...74999]
   ▶ [75000...99999]

However, it seems Object Inspector attempts to display the whole list or properties or something like that, because it freezes my browser.
Whiteboard: [console-html] [triage]
Hi Oriol, thanks for reporting.

This seems like https://github.com/devtools-html/devtools-core/issues/538 and will be fixed in he 0.12 reps bundle
Depends on: 1386525
Flags: qe-verify?
Priority: -- → P3
Whiteboard: [console-html] [triage] → [reserve-console-html]
Assignee: nobody → nchevobbe
Status: NEW → ASSIGNED
Priority: P3 → P1
Iteration: --- → 57.1 - Aug 15
Iteration: 57.1 - Aug 15 → 57.2 - Aug 29
See Also: → 1391077
Iteration: 57.2 - Aug 29 → 57.3 - Sep 19
Depends on: 1399460
Does not lag anymore with Bug 1399460. Can you confirm Oriol ?
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Flags: needinfo?(oriol-bugzilla)
Resolution: --- → WORKSFORME
Assignee: nchevobbe → nobody
Iteration: 57.3 - Sep 19 → ---
Flags: qe-verify?
Priority: P1 → --
Whiteboard: [reserve-console-html]
Now it's better for new Uint8Array(1e5), but if I use new Uint8Array(1e9) Firefox still freezes and eats up all my 16GB of RAM.
Flags: needinfo?(oriol-bugzilla)
Yeah, I reproduce as well (as well as in the variable view).
I am not sure what is going on here, and the fact that it crashes the devtools does not help.
I am going to re-open this bug and investigate more.
Status: RESOLVED → REOPENED
Resolution: WORKSFORME → ---
Status: REOPENED → NEW
status-firefox56: --- → unaffected
status-firefox57: --- → fix-optional
Whiteboard: [console-html][triage]
I thinks this might come from http://searchfox.org/mozilla-central/source/devtools/server/actors/object.js#898,901.
We call this when we want to retrieve all the non-indexed properties of an array. But here getOwnPropertyNames is really costly with such a big array (in fact it crashes the browser).
I think Debugger.Object needs a new method to query own properties without allocating a huge array.
Flags: qe-verify+
Priority: -- → P3
QA Contact: iulia.cristescu
Whiteboard: [console-html][triage] → [reserve-console-html]
We are meaning to skip the call for TypedArrays altogether in Bug 1348761, but I guess we may have missed a spot and are calling getOwnPropertyNames directly. Still, it's not ideal to need to skip the call for TypedArrays, and I was wondering if there might be some optimization on the JS side for this. For example, I get anywhere from 1.5-2.5 seconds with this test case:

data:text/html,<body><script>let start = performance.now();Object.getOwnPropertyNames(new Uint8Array(1e7));document.body.innerHTML = (performance.now() - start) + "ms";</script></body>

So it's definitely going to cause problems in the frontend when we are trying to preview objects like this.

Jim, do you have ideas (or know who we could ask) about optimizing the call to getOwnPropertyNames, or providing some kind of D.O. API that lets us get the info we want more efficiently?
Flags: needinfo?(jimb)
See Also: → 1348761
(In reply to Brian Grinstead [:bgrins] from comment #7)
> For example, I get anywhere from 1.5-2.5 seconds with this test case:
> 
> data:text/html,<body><script>let start =
> performance.now();Object.getOwnPropertyNames(new
> Uint8Array(1e7));document.body.innerHTML = (performance.now() - start) +
> "ms";</script></body>

FWIW on Chrome 63 this test case takes 5-6 seconds for me
The specific case we're hitting is here : http://searchfox.org/mozilla-central/source/devtools/server/actors/object.js#898,901,907

We want to be able to retrieve non-indexed properties on arrays (length, buffer, bufferOffset, custom-set properties; i.e. `x = []; x.foo ="bar"`, …).
In the function, it uses getOwnPropertyNames, find the length property and cut the array after that index. For large typed arrays, this takes too much time, but i'm not aware of any way to only get those non-indexed properties in a fast way.
(In reply to Oriol Brufau [:Oriol] from comment #6)
> I think Debugger.Object needs a new method to query own properties without
> allocating a huge array.

Agreed, that would be the more helpful approach. The whole point of Debugger.Object is to facilitate this sort of inspection. I'm busy today, but I'll try to draft an alternative interface tomorrow. Leaving the needinfo? jimb flag set.

Is simply excluding indexed properties enough?

Should there be some sort of cap on the number of named properties returned as well? That would mean that you'd need some way to retrieve selected subsets of property names, so you had the option of iterating through all of them, if you wanted.

It's not too hard to imagine what sort of user experience browsing a gigantic array should have: the ability to jump to a given address, see dumps of a portion of the array around a given address, scroll back and forth.

What's the best experience for browsing an object with many, many named properties? We'd probably need to sort them, and provide ranges?
(In reply to Jim Blandy :jimb from comment #10)
> What's the best experience for browsing an object with many, many named
> properties? We'd probably need to sort them, and provide ranges?

Oh, a live search might be good. Facilitated by a D.O.p.searchNamedProperties(regex or substr) function?
Flags: needinfo?(jimb)
live search w/ a regex seems good, this way it would be flexible
Resetting the needinfo since I think it was accidentally cleared
Flags: needinfo?(jimb)
(In reply to Jim Blandy :jimb from comment #11)
> (In reply to Jim Blandy :jimb from comment #10)
> > What's the best experience for browsing an object with many, many named
> > properties? We'd probably need to sort them, and provide ranges?

Sounds sort of like our existing array bucketing but with sorted keys - which could definitely work. Although it would change the normal enumeration order so we'd just have to decide if that's worth the performance trade off. We should also look into how CDP handles this interaction to see if it might make sense to copy.

> Oh, a live search might be good. Facilitated by a
> D.O.p.searchNamedProperties(regex or substr) function?

We've talked about having a search box either inline on the OI or providing a sidebar like the variables view (bug 1308566).

Object search in general is tricky, but we do have an implementation for the variables view (with some limitations about the depth that it will search, cyclical objects, etc). I think Alex knows this code better, so asking him if a new D.O. API might help with some of the limitations.
Flags: needinfo?(poirot.alex)
(In reply to Brian Grinstead [:bgrins] from comment #14)
> (In reply to Jim Blandy :jimb from comment #11)
> > (In reply to Jim Blandy :jimb from comment #10)
> > > What's the best experience for browsing an object with many, many named
> > > properties? We'd probably need to sort them, and provide ranges?
> 
> Sounds sort of like our existing array bucketing but with sorted keys -
> which could definitely work. Although it would change the normal enumeration
> order so we'd just have to decide if that's worth the performance trade off.
> We should also look into how CDP handles this interaction to see if it might
> make sense to copy.

The variable view used to sort and display by chunks.
If there is more than 2k properties, we split them in 4 chunks.
It used to replicate what chrome was doing.

> > Oh, a live search might be good. Facilitated by a
> > D.O.p.searchNamedProperties(regex or substr) function?
> 
> We've talked about having a search box either inline on the OI or providing
> a sidebar like the variables view (bug 1308566).

If we want to replicate VariableView filter behavior, which also look into values.
I'm not sure having per property *names* filter implemented in C++ be any useful.
We will have to retrieve all of them to look into all values.
It may only be useful if we only search per property names and that, only for the inspector object (and so, none of its property values).
Unless searchNamedProperties's regex/substr was also looking into values like what this js code does:
http://searchfox.org/mozilla-central/rev/a4702203522745baff21e519035b6c946b7d710d/devtools/server/actors/object.js#950-960
Flags: needinfo?(poirot.alex)
Depends on: 1405509
(In reply to Alexandre Poirot [:ochameau] from comment #15)
> I'm not sure having per property *names* filter implemented in C++ be any
> useful.

I think this is right. In bug 1405509, I noted that, when an object has many named properties, D.O.p.getOwnPropertyNames indeed only allocates the array to hold them, and shares the property name strings with the object itself. It does build the array and make a few passes over it, so if it shows up in timing profiles then we could revisit this, but I would expect that to be swamped by the cost of actually filtering the elements for display.

For massive arrays, it's hard for me to believe that chunks are actually what users want to see. A scrollable view, with a way to specify the exact index one wants, seems much more useful. Perhaps one element per line for arrays of JS objects, or several elements per line for arrays of numbers, including typed arrays. It would be like looking at a memory dump (which is exactly what it is for Wasm).

Moving the needinfo to 1405509.
Priority: P3 → P2
Whiteboard: [reserve-console-html] → [newconsole-mvp]
Flags: qe-verify+
QA Contact: iulia.cristescu
(Clearing needinfo, since this is blocked on bug 1405509, to which I'm assigned.)
Flags: needinfo?(jimb)
Whiteboard: [newconsole-mvp]
Product: Firefox → DevTools
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.