Open Bug 1392562 Opened 7 years ago Updated 2 years ago

generateCssProperties is slow

Categories

(DevTools :: Inspector, defect, P3)

defect

Tracking

(firefox57 fix-optional)

Tracking Status
firefox57 --- fix-optional

People

(Reporter: ochameau, Unassigned)

References

(Blocks 1 open bug)

Details

(Whiteboard: [designer-tools])

http://searchfox.org/mozilla-central/source/devtools/server/actors/css-properties.js#48-81 This method frequently appears in profiles: https://perfht.ml/2x85rCF It takes 90ms on my VM and if you clear the filter on perf-html you can see it as a significant pause into inspector loading sequence.
Priority: -- → P3
It seems to be mostly because of this code: http://searchfox.org/mozilla-central/rev/999385a5e8c2d360cc37286882508075fc2078bd/devtools/server/actors/css-properties.js#56-67 for (let type in CSS_TYPES) { if (safeCssPropertySupportsType(name, DOMUtils["TYPE_" + type])) { supports.push(CSS_TYPES[type]); } } // Don't send colors over RDP, these will be re-attached by the front. let values = DOMUtils.getCSSValuesForProperty(name); if (values.includes("aliceblue")) { values = values.filter(x => !colors.includes(x)); values.unshift("COLOR"); } The color filter is so dumb, but I don't know how to optimize this... Tom, Any idea how to tweak that? Should we also computed a static file??
Flags: needinfo?(ttromey)
(In reply to Alexandre Poirot [:ochameau] from comment #1) > The color filter is so dumb, but I don't know how to optimize this... This is a primitive form of compression. The colors are "re-attached" on the client. IIRC Greg tried this both ways and this was faster...? > Tom, Any idea how to tweak that? Should we also computed a static file?? I think he also attempted this but there was no way to integrate this into the build system. The ideal is not to do any of this stuff at all but rather change the inspector to query the actor. However, this also proved to be intractable. Two ideas come to mind. One is, fix the inspector. Like I said, it's hard, but maybe now there's the impetus. This is the best route. The other idea is pushing this compression idea into inIDOMUtils and have it emit some special token into the results to mean "colors are valid here"; then continue with the re-attachment in the client. Gross! But it would work.
Flags: needinfo?(ttromey) → needinfo?(gtatum)
(In reply to Tom Tromey :tromey from comment #2) > (In reply to Alexandre Poirot [:ochameau] from comment #1) > > > The color filter is so dumb, but I don't know how to optimize this... > > This is a primitive form of compression. The colors are "re-attached" on > the client. > IIRC Greg tried this both ways and this was faster...? I was wondering if we could only change the filtering algorithm. May be Array.filter is slower than for()? May be there is super-smart way to filter out color... somehow? Or get the native API to not return the color if that's easy for it to do? > I think he also attempted this but there was no way to integrate this into > the build system. > > The ideal is not to do any of this stuff at all but rather change the > inspector to > query the actor. However, this also proved to be intractable. > > Two ideas come to mind. > > One is, fix the inspector. Like I said, it's hard, but maybe now there's > the impetus. > This is the best route. Thanks for the history telling ;) Yes it sounds hard, I tried to track down cssProperties, but by itself it already draws quite complex codepaths!
I don't really trust my old measurements, especially since that was before perf.html. Because of all the compile flags that go into the CSS system, it's impossible for our build system to generate a declarative list, so we have to ask. The biggest thing is getting the correct set of supported CSS properties on the given thing that we are inspecting. Is there a way for the actor to send over a version number for the build and then cache our database on the version? That would mean we compute it once and then re-use it.
Flags: needinfo?(gtatum)
I re-ran a test of the async loading in the front, doing runs one with each approach. They are really in the same range. perf-html loading screen Sending full colors: 108ms Sending full colors: 70ms Sending full colors: 58ms Sending full colors: 81ms Sending full colors: 87ms Shaving off colors: 76ms Shaving off colors: 64ms Shaving off colors: 69ms Shaving off colors: 67ms Shaving off colors: 68ms cnn.com Sending full colors: 324ms Sending full colors: 445ms Sending full colors: 549ms Sending full colors: 247ms Shaving off colors: 159ms Shaving off colors: 215ms Shaving off colors: 233ms Shaving off colors: 514ms Shaving off colors: 441ms
Brad: This is something that platform knowledge may help us optimize. I figured I'd add it to your radar.
Product: Firefox → DevTools
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.