Shipping Oilpan for core/animations

[Kentaro Hara]

Hi

[Summary] We're planning to ship Oilpan for core/animations except for temporary objects such as AnimatableValues. We're planning to land https://codereview.chromium.org/1318543009/. It won't cause any performance regression.

[Details]

Performance results are as follows:

Linux: https://257e68df4dae930120a4c67b6f7a507d5042fbc3.googledrive.com/host/0B4aiM9jljUy8UEh0QjR3SUVXaVE/vanilla-vs-part-vs-full.html

Nexus4 & Nexus7: https://aef6e225fbf3eabf04ab37cea7bc6fd8766d0e78-www.googledrive.com/host/0B6XhrzUCq5lhUjloNWR3b0xxRXM/2015/partanim.html

*-vanilla: ToT

*-vanilla-partanim: ToT + ship oilpan for core/animations except for temporary objects

*-vanilla-fullanim: ToT + ship oilpan for everything in core/animations

The highlights are as follows:

- The important numbers are tough_animation_cases.frame_times and tough_animation_cases.queueing_durations for each benchmark.

- There is no problematic regression in linux-vanilla-partanim, n4-vanilla-partanim and n7-vanilla-partanim. This means that there is no regression in shipping oilpan for core/animations except for temporary objects. Our proposal is to do this.

- linux-vanilla-fullanim hits a 13 - 18% regression at tough_animation_cases.frame_times of balls_css_properties_* benchmarks. We confirmed that this is caused by a pause time of a conservative GC which has to collect a ton of temporary animation objects such as AnimatableValues and InterporableValues. We didn't observe similar regressions in Nexus4 and Nexus7. We confirmed that this is because Nexus4 and Nexus7 are slow and thus the pause time of conservative GCs don't affect the frame_times metric.

It is unfortunate that we cannot ship Oilpan for temporary animation objects, but I think that this would be a fair decision for the following reasons:

- The balls_css_properties_* benchmarks allocate an incredible number of AnimatableValues & InterpolableValues. If we put those temporary objects on Oilpan's heap, they add a significant pressure on the heap.

- There is no advantage in putting those temporary objects on Oilpan's heap because their lifetime is very clear.

- I have tried a bunch of approaches to resolve the regression but concluded that we won't be able to resolve the regression without introducing an incremental marking or/and a generational GC to Oilpan. I don't think that the engineering cost and the complexity outweighs the benefit. At the very least, it won't be a task of Oilpan v1.

- For core/animations/ and Oilpan, it is more important to ship Oilpan for core animation objects such as Animation, AnimationTimeline, ElementAnimation, DocumentTimeline, SampledEffect etc. Oilpan solves the complicated lifetime problem. Actually, https://codereview.chromium.org/1318543009/ removes a bunch of complexities to clear raw pointers between these objects.

In conclusion, our proposal is:

- ship Oilpan for core/animations except for temporary objects (i.e., land https://codereview.chromium.org/1318543009/).

- unship Oilpan from the temporary objects (i.e., land https://codereview.chromium.org/1276183004/)

We confirmed that it won't cause any performance regression in the tough benchmarks in telemetry.

Does the plan sound good to you?

Thanks!

--

Kentaro Hara, Tokyo, Japan

[Alan Cutter]

No animation performance regressions sounds good to me. (:

All those raw pointers look a bit scary but they all appear to be stack allocated so that's okay.

LGTM to ship, I will review the patches in detail.

I'm curious what the view is on when to Oilpan and when not to. Are we trying to Oilpan as much as we can without performance regression for the benefit of pushing deallocation into scheduled idle time?

[Alan Cutter]

+sh...@chromium.org

[Kentaro Hara]

I'm curious what the view is on when to Oilpan and when not to. Are we trying to Oilpan as much as we can without performance regression for the benefit of pushing deallocation into scheduled idle time?

When we started Oilpan, we were planning to move everything to Oilpan, including all RefPtrs, OwnPtrs, Strings etc. Later we realized that that wouldn't be a good plan for performance.

The latest plan is as follows:

a) Objects that have a complex lifetime should be moved to Oilpan. This is important to implement correct lifetime relationships and decrease use-after-frees. Examples are Nodes, all DOM objects that have wrappers, various animation objects etc.

b) Objects that are heavily allocated and have a clear lifetime should not be moved to Oilpan. Examples are AnimatableValues, InterporableValues, Strings etc.

c) For other objects, it depends on situations. Examples are LayoutObjects, CSSValues etc. We're planning to move CSSValues to Oilpan because putting CSSValues out-of-Oilpan increases # of Persistent handles. We're not planning to move LayoutObjects to Oilpan because LayoutObjects have a very clear lifetime and we don't see any benefit in moving them to Oilpan.

[Elliott Sprehn]

Where are the persistent handles coming from?

[Kentaro Hara]

On Tue, Sep 1, 2015 at 4:49 PM, Elliott Sprehn <esp...@chromium.org> wrote:

Where are the persistent handles coming from?

Yeah, as you mentioned, I think we can drop the number of persistent handles by:

- unshipping Oilpan from StyleImage and StylePropertySet

- unshipping Oilpan from CSS-related classes such as CSSExpressionNode etc

I'd prefer keeping CSSValues on Oilpan because it reduces a bunch of raw pointers to CSSValues (and thus improves security) but I don't have a strong opinion on this.