1443314 - Consider removing the svg.path-caching.enabled pref

Status: RESOLVED FIXED

(Core :: Layout, enhancement)

Description

[Nathan Froyd [:froydnj]]

svg.path-caching.enabled was flipped to true in bug 932762.

svg.display-lists.{hit-testing,painting}.enabled were set to true in bug 776054.

The former is 3.5 years old, the latter about 5.5.  I suspect that we've rarely tested with those prefs set to false over the past several years.

Could we declare victory and just remove those prefs (and their associated caches) entirely?  dholbert, do you know who would be the best person to make that call?

Comment 1

[Daniel Holbert [:dholbert]]

Indeed, we can probably remove these prefs. jwatt, sound good to you?

Comment 2

[Jonathan Watt [:jwatt]]

Thanks for looking for dead prefs. Is there much of a reason to remove them other than just to clean things up? Do we have so many content prefs to pass over early IPC that it's causing issues, for example?

See bug 829802 for the display list prefs.

Regarding svg.path-caching.enabled, the very limited profiling that I did after path caching was added indicated that it wasn't much of a win, although Bas was surprised about that and wanted more testing. That never happened since we were very busy around that time. Ideally we should do that though, and remove path caching itself if we don't see it helping since otherwise it just needlessly increases our memory use.

That said, we would do that with a hacked build, so I'm fine with removing this pref.

Comment 3

[Nathan Froyd [:froydnj]]

(In reply to Jonathan Watt [:jwatt] (needinfo? me) from comment #2)
> Thanks for looking for dead prefs. Is there much of a reason to remove them
> other than just to clean things up? Do we have so many content prefs to pass
> over early IPC that it's causing issues, for example?

This came out of examining nsLayoutStatics as part of looking at our initialization process and trying to understand what everything was doing.  A lot of code called from nsLayoutStatics seems to be in the business of setting up pref caches and the like.

I don't think there's much reason to remove them other than cleanup, but cleaning up options that are always on and aren't likely to be flipped off is a nice thing to do in general.

Comment 4

[Nicholas Nethercote [inactive]]

"just to clean things up" is a perfectly valid reason to remove prefs. We have over 3000 prefs, many of which are likely unnecessary and should be removed.

Comment 5

[Robert Longson [:longsonr]]

Attachment: patch

Comment 6

[Jonathan Watt [:jwatt]]

(In reply to Nicholas Nethercote [:njn] from comment #4)
> "just to clean things up" is a perfectly valid reason to remove prefs.

Absolutely. I wanted to know if there were additional reasons I should be aware of.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/4ae5b14ff800
remove the svg.path-caching.enabled pref r=jwatt

Comment 8

[Nicholas Nethercote [inactive]]

> Absolutely. I wanted to know if there were additional reasons I should be
> aware of.

This Google Doc has some info about that:
https://docs.google.com/document/d/1V5Wc9bXwfgMG2JOsswvDPVwZl_xiaSBxwJXf3fiEaU8/edit#

Comment 9

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/4ae5b14ff800