[Oilpan] Migrate most classes under core/animations to Oilpan heap.

Migrate most classes under core/animations to Oilpan heap.


BUG=479467, 497660, 497595

[peria, 2015-05-01 07:50:26]

peria@chromium.org changed reviewers:
+ haraken@chromium.org, kouhei@chromium.org, oilpan-reviews@chromium.org

[peria, 2015-05-01 07:50:28]

Not ready for review, currently.

I'm trying to run tests and to measure performance.

https://codereview.chromium.org/1120003002/diff/1/Source/core/dom/ElementRare...
File Source/core/dom/ElementRareData.cpp (right):

https://codereview.chromium.org/1120003002/diff/1/Source/core/dom/ElementRare...
Source/core/dom/ElementRareData.cpp:46: #endif
NOTE:
With this change, the size of ElementRareData class becomes 200 Bytes from 160
Bytes, on non-Oilpan build.
It will be back to 160 Bytes if ElementRareData moves to Oilpan heap.

[haraken, 2015-05-01 09:04:07]

Yes, let's measure the performance implication of this change!

https://codereview.chromium.org/1120003002/diff/1/Source/core/dom/ElementRare...
File Source/core/dom/ElementRareData.cpp (right):

https://codereview.chromium.org/1120003002/diff/1/Source/core/dom/ElementRare...
Source/core/dom/ElementRareData.cpp:46: #endif
On 2015/05/01 07:50:28, peria wrote:
> NOTE:
> With this change, the size of ElementRareData class becomes 200 Bytes from 160
> Bytes, on non-Oilpan build.
> It will be back to 160 Bytes if ElementRareData moves to Oilpan heap.

This is because sizeof(Persistent) is too large. I'm trying to reduce the size.

[haraken, 2015-05-08 00:13:07]

Be careful when droppoing #if !ENABLE(OILPAN). We cannot mechanically drop the
#if macros until we ship Oilpan for everything.

(I haven't checked all #if macros in this CL, but please just check if you can
remove or have to keep them.)

https://codereview.chromium.org/1120003002/diff/40001/Source/core/animation/A...
File Source/core/animation/Animation.cpp (left):

https://codereview.chromium.org/1120003002/diff/40001/Source/core/animation/A...
Source/core/animation/Animation.cpp:88:
m_target->ensureElementAnimations().addAnimation(this);

You cannot remove this.

https://codereview.chromium.org/1120003002/diff/40001/Source/core/animation/A...
Source/core/animation/Animation.cpp:96:
m_target->elementAnimations()->notifyAnimationDestroyed(this);

Ditto.

https://codereview.chromium.org/1120003002/diff/40001/Source/core/animation/A...
Source/core/animation/Animation.cpp:227: void
Animation::notifyElementDestroyed()

Ditto.

https://codereview.chromium.org/1120003002/diff/40001/Source/core/animation/A...
File Source/core/animation/Animation.h (left):

https://codereview.chromium.org/1120003002/diff/40001/Source/core/animation/A...
Source/core/animation/Animation.h:74: void notifyElementDestroyed();

Given that we've not yet shipped oilpan for the Node hierarchy (i.e., the
Element on which the animation is attached), you cannot remove this code at this
point.

https://codereview.chromium.org/1120003002/diff/40001/Source/core/animation/A...
File Source/core/animation/AnimationTimeline.cpp (left):

https://codereview.chromium.org/1120003002/diff/40001/Source/core/animation/A...
Source/core/animation/AnimationTimeline.cpp:311: void
AnimationTimeline::detachFromDocument()

Since Document is not yet moved to oilpan by default, this code is still needed.

https://codereview.chromium.org/1120003002/diff/40001/Source/core/animation/A...
File Source/core/animation/AnimationTimeline.h (left):

https://codereview.chromium.org/1120003002/diff/40001/Source/core/animation/A...
Source/core/animation/AnimationTimeline.h:106: void detachFromDocument();

Ditto.

https://codereview.chromium.org/1120003002/diff/40001/Source/core/animation/E...
File Source/core/animation/ElementAnimations.cpp (left):

https://codereview.chromium.org/1120003002/diff/40001/Source/core/animation/E...
Source/core/animation/ElementAnimations.cpp:47:
animation->notifyElementDestroyed();

Ditto.

https://codereview.chromium.org/1120003002/diff/40001/Source/core/animation/E...
File Source/core/animation/ElementAnimations.h (left):

https://codereview.chromium.org/1120003002/diff/40001/Source/core/animation/E...
Source/core/animation/ElementAnimations.h:79: void
notifyAnimationDestroyed(Animation* animation) {
m_animations.remove(m_animations.find(animation)); }

Ditto.

https://codereview.chromium.org/1120003002/diff/40001/Source/core/animation/E...
Source/core/animation/ElementAnimations.h:96: Vector<Animation*> m_animations;

Ditto.

[peria, 2015-05-08 02:21:20]

This mail is just for an acknowledgement.
I did nothing for those #if macros yet in PS4.

# Many classes are renamed in PS4 and latter patch sets,
# because of http://crrev.com/1113173003

https://codereview.chromium.org/1120003002/diff/40001/Source/core/animation/A...
File Source/core/animation/Animation.cpp (left):

https://codereview.chromium.org/1120003002/diff/40001/Source/core/animation/A...
Source/core/animation/Animation.cpp:88:
m_target->ensureElementAnimations().addAnimation(this);
On 2015/05/08 00:13:06, haraken wrote:
> 
> You cannot remove this.

Acknowledged.

https://codereview.chromium.org/1120003002/diff/40001/Source/core/animation/A...
Source/core/animation/Animation.cpp:96:
m_target->elementAnimations()->notifyAnimationDestroyed(this);
On 2015/05/08 00:13:06, haraken wrote:
> 
> Ditto.

Acknowledged.

https://codereview.chromium.org/1120003002/diff/40001/Source/core/animation/A...
Source/core/animation/Animation.cpp:227: void
Animation::notifyElementDestroyed()
On 2015/05/08 00:13:06, haraken wrote:
> 
> Ditto.

Acknowledged.

https://codereview.chromium.org/1120003002/diff/40001/Source/core/animation/A...
File Source/core/animation/Animation.h (left):

https://codereview.chromium.org/1120003002/diff/40001/Source/core/animation/A...
Source/core/animation/Animation.h:74: void notifyElementDestroyed();
On 2015/05/08 00:13:06, haraken wrote:
> 
> Given that we've not yet shipped oilpan for the Node hierarchy (i.e., the
> Element on which the animation is attached), you cannot remove this code at
this
> point.
> 

Thank you for this notification.
I'll follow inside of those #if OILPAN macros and revert some changes.

https://codereview.chromium.org/1120003002/diff/40001/Source/core/animation/A...
File Source/core/animation/AnimationTimeline.cpp (left):

https://codereview.chromium.org/1120003002/diff/40001/Source/core/animation/A...
Source/core/animation/AnimationTimeline.cpp:311: void
AnimationTimeline::detachFromDocument()
On 2015/05/08 00:13:06, haraken wrote:
> 
> Since Document is not yet moved to oilpan by default, this code is still
needed.

Acknowledged.

https://codereview.chromium.org/1120003002/diff/40001/Source/core/animation/A...
File Source/core/animation/AnimationTimeline.h (left):

https://codereview.chromium.org/1120003002/diff/40001/Source/core/animation/A...
Source/core/animation/AnimationTimeline.h:106: void detachFromDocument();
On 2015/05/08 00:13:06, haraken wrote:
> 
> Ditto.

Acknowledged.

https://codereview.chromium.org/1120003002/diff/40001/Source/core/animation/E...
File Source/core/animation/ElementAnimations.cpp (left):

https://codereview.chromium.org/1120003002/diff/40001/Source/core/animation/E...
Source/core/animation/ElementAnimations.cpp:47:
animation->notifyElementDestroyed();
On 2015/05/08 00:13:06, haraken wrote:
> 
> Ditto.

Acknowledged.

https://codereview.chromium.org/1120003002/diff/40001/Source/core/animation/E...
File Source/core/animation/ElementAnimations.h (left):

https://codereview.chromium.org/1120003002/diff/40001/Source/core/animation/E...
Source/core/animation/ElementAnimations.h:79: void
notifyAnimationDestroyed(Animation* animation) {
m_animations.remove(m_animations.find(animation)); }
On 2015/05/08 00:13:06, haraken wrote:
> 
> Ditto.

Acknowledged.

https://codereview.chromium.org/1120003002/diff/40001/Source/core/animation/E...
Source/core/animation/ElementAnimations.h:96: Vector<Animation*> m_animations;
On 2015/05/08 00:13:06, haraken wrote:
> 
> Ditto.

Acknowledged.

[haraken, 2015-05-08 02:26:02]

On 2015/05/08 02:21:20, peria wrote:
> This mail is just for an acknowledgement.
> I did nothing for those #if macros yet in PS4.
> 
> # Many classes are renamed in PS4 and latter patch sets,
> # because of http://crrev.com/1113173003
> 
>
https://codereview.chromium.org/1120003002/diff/40001/Source/core/animation/A...
> File Source/core/animation/Animation.cpp (left):
> 
>
https://codereview.chromium.org/1120003002/diff/40001/Source/core/animation/A...
> Source/core/animation/Animation.cpp:88:
> m_target->ensureElementAnimations().addAnimation(this);
> On 2015/05/08 00:13:06, haraken wrote:
> > 
> > You cannot remove this.
> 
> Acknowledged.
> 
>
https://codereview.chromium.org/1120003002/diff/40001/Source/core/animation/A...
> Source/core/animation/Animation.cpp:96:
> m_target->elementAnimations()->notifyAnimationDestroyed(this);
> On 2015/05/08 00:13:06, haraken wrote:
> > 
> > Ditto.
> 
> Acknowledged.
> 
>
https://codereview.chromium.org/1120003002/diff/40001/Source/core/animation/A...
> Source/core/animation/Animation.cpp:227: void
> Animation::notifyElementDestroyed()
> On 2015/05/08 00:13:06, haraken wrote:
> > 
> > Ditto.
> 
> Acknowledged.
> 
>
https://codereview.chromium.org/1120003002/diff/40001/Source/core/animation/A...
> File Source/core/animation/Animation.h (left):
> 
>
https://codereview.chromium.org/1120003002/diff/40001/Source/core/animation/A...
> Source/core/animation/Animation.h:74: void notifyElementDestroyed();
> On 2015/05/08 00:13:06, haraken wrote:
> > 
> > Given that we've not yet shipped oilpan for the Node hierarchy (i.e., the
> > Element on which the animation is attached), you cannot remove this code at
> this
> > point.
> > 
> 
> Thank you for this notification.
> I'll follow inside of those #if OILPAN macros and revert some changes.

I guess this is (at least one of the) causes of the test failures.

[peria, 2015-05-11 10:27:54]

haraken@,
I wonder why MockPlatformTiming is traced in TearDown.
Looking the following test result, in non-Oipan build, Heap::CollectAllGarbage()
in AnimationAnimationTimelineTest::TearDown traces
PersistentAnchor -> Persistent<AnimationTimeline> -> MockPlatformTiming,
while the Persistent<AnimationTimeline> should have been released and its
members should not be traced.
(It does not happen in Oilpan build.)

What do I mis-understand?

http://build.chromium.org/p/tryserver.blink/builders/linux_blink_dbg/builds/4...

https://codereview.chromium.org/1120003002/diff/160001/Source/core/animation/...
File Source/core/animation/AnimationTimelineTest.cpp (right):

https://codereview.chromium.org/1120003002/diff/160001/Source/core/animation/...
Source/core/animation/AnimationTimelineTest.cpp:85: *foo = 1;
This change is made to share how an instance of MockPlatformTiming is traced in
a test, and will be reverted soon.

[haraken, 2015-05-11 13:24:56]

> I wonder why MockPlatformTiming is traced in TearDown.
> Looking the following test result, in non-Oipan build,
Heap::CollectAllGarbage()
> in AnimationAnimationTimelineTest::TearDown traces
> PersistentAnchor -> Persistent<AnimationTimeline> -> MockPlatformTiming,
> while the Persistent<AnimationTimeline> should have been released and its
> members should not be traced.
> (It does not happen in Oilpan build.)

Hmm, this looks strange indeed.

Are you sure that the traced Persistent<AnimationTimeline> is equal to the
AnimationAnimationTimelineTest::timeline? I mean, isn't there any other
Persistent<AnimationTimeline> around?

You can hook Persistent's constructor to see how many
Persistent<AnimationTimeline>s are created in the test.

[peria, 2015-05-27 06:28:08]

Maybe now it's ready for review.
Please take a look.

[haraken, 2015-05-27 07:06:16]

On 2015/05/27 06:28:08, peria wrote:
> Maybe now it's ready for review.
> Please take a look.

This sounds great!

Can you get performance & memory numbers on mobile working with yuta-san?

[peria, 2015-05-28 02:37:16]

peria@chromium.org changed reviewers:
+ yutak@chromium.org

[peria, 2015-05-28 02:37:18]

Yuta-san,
Could you measure performance of following tests with and without applying this
CL(PS 18) on Nexus 4&7?
- blink_perf.animation
- blink_perf.layout
- smoothness.tough_animation_case
- memory.top_7_stress
- memory.top_7_stress_slimming_paint

[haraken, 2015-05-28 02:41:10]

On 2015/05/28 02:37:18, peria wrote:
> Yuta-san,
> Could you measure performance of following tests with and without applying
this
> CL(PS 18) on Nexus 4&7?
> - blink_perf.animation
> - blink_perf.layout
> - smoothness.tough_animation_case
> - memory.top_7_stress
> - memory.top_7_stress_slimming_paint

Will it be better to run those benchmarks in your Linux desktop and see the
performance data before investigating mobile in details?

[peria, 2015-05-28 05:34:34]

On 2015/05/28 02:41:10, haraken wrote:
> On 2015/05/28 02:37:18, peria wrote:
> > Yuta-san,
> > Could you measure performance of following tests with and without applying
> this
> > CL(PS 18) on Nexus 4&7?
> > - blink_perf.animation
> > - blink_perf.layout
> > - smoothness.tough_animation_case
> > - memory.top_7_stress
> > - memory.top_7_stress_slimming_paint
> 
> Will it be better to run those benchmarks in your Linux desktop and see the
> performance data before investigating mobile in details?

Yes, I ran them for few times on my Linux.
Only smoothness.tough_animation_cases:responsive-biggest_jank_thread_time
seems to regress ~10%, but the data is unreliable.
Other tests seem to keep as-is or regress < 5%.

[sof, 2015-05-30 11:34:01]

sigbjornf@opera.com changed reviewers:
+ sigbjornf@opera.com

[sof, 2015-05-30 11:34:05]

https://codereview.chromium.org/1120003002/diff/320001/Source/core/animation/...
File Source/core/animation/Animation.h (right):

https://codereview.chromium.org/1120003002/diff/320001/Source/core/animation/...
Source/core/animation/Animation.h:58: USING_PRE_FINALIZER(Animation,
disposeAnimation);
No need to add a TODO, but eagerly finalizing Animation objects will be an
alternative.

https://codereview.chromium.org/1120003002/diff/320001/Source/core/animation/...
File Source/core/animation/AnimationStack.cpp (left):

https://codereview.chromium.org/1120003002/diff/320001/Source/core/animation/...
Source/core/animation/AnimationStack.cpp:91: // std::sort doesn't work with
OwnPtrs
nit: s/OwnPtr/Member/ (or remove the comment, doesn't add much.)

https://codereview.chromium.org/1120003002/diff/320001/Source/core/animation/...
File Source/core/animation/AnimationTimeline.cpp (right):

https://codereview.chromium.org/1120003002/diff/320001/Source/core/animation/...
Source/core/animation/AnimationTimeline.cpp:138: HeapVector<Member<Animation>>
animations;
Use copyToVector() instead?

https://codereview.chromium.org/1120003002/diff/320001/Source/core/animation/...
File Source/core/animation/AnimationTimeline.h (right):

https://codereview.chromium.org/1120003002/diff/320001/Source/core/animation/...
Source/core/animation/AnimationTimeline.h:55: class PlatformTiming : public
GarbageCollectedFinalized<PlatformTiming> {
Unusual abstract interface, it also deriving from a GC base class.

https://codereview.chromium.org/1120003002/diff/320001/Source/core/animation/...
File Source/core/animation/KeyframeEffect.h (right):

https://codereview.chromium.org/1120003002/diff/320001/Source/core/animation/...
Source/core/animation/KeyframeEffect.h:67: const EffectModel* model() const {
return m_model.get(); }
nit: redundant get()

https://codereview.chromium.org/1120003002/diff/320001/Source/core/animation/...
File Source/core/animation/LengthBoxStyleInterpolation.cpp (right):

https://codereview.chromium.org/1120003002/diff/320001/Source/core/animation/...
Source/core/animation/LengthBoxStyleInterpolation.cpp:107:
StyleInterpolation::trace(visitor);
nit: move this to the end?

https://codereview.chromium.org/1120003002/diff/320001/Source/core/animation/...
File Source/core/animation/SampledEffect.h (right):

https://codereview.chromium.org/1120003002/diff/320001/Source/core/animation/...
Source/core/animation/SampledEffect.h:29: HeapVector<Member<Interpolation>>*
mutableInterpolations() { return m_interpolations.get(); }
get() needed?

https://codereview.chromium.org/1120003002/diff/320001/Source/core/animation/...
File Source/core/animation/StringKeyframe.cpp (right):

https://codereview.chromium.org/1120003002/diff/320001/Source/core/animation/...
Source/core/animation/StringKeyframe.cpp:277: break;
Could you fix indentation of this "break"?

https://codereview.chromium.org/1120003002/diff/320001/Source/core/animation/...
File Source/core/animation/animatable/AnimatableLengthPoint.h (right):

https://codereview.chromium.org/1120003002/diff/320001/Source/core/animation/...
Source/core/animation/animatable/AnimatableLengthPoint.h:45: const
AnimatableValue* x() const { return m_x.get(); }
redundant get()s.

https://codereview.chromium.org/1120003002/diff/320001/Source/core/animation/...
File Source/core/animation/animatable/AnimatableLengthPoint3D.h (right):

https://codereview.chromium.org/1120003002/diff/320001/Source/core/animation/...
Source/core/animation/animatable/AnimatableLengthPoint3D.h:45: const
AnimatableValue* x() const { return m_x.get(); }
yet more redundant get()s

https://codereview.chromium.org/1120003002/diff/320001/Source/core/animation/...
File Source/core/animation/animatable/AnimatableLengthSize.h (right):

https://codereview.chromium.org/1120003002/diff/320001/Source/core/animation/...
Source/core/animation/animatable/AnimatableLengthSize.h:45: const
AnimatableValue* width() const { return m_width.get(); }
get() redundant - could you take a pass through the header files touched here
and remove them where possible?

https://codereview.chromium.org/1120003002/diff/320001/Source/core/animation/...
File Source/core/animation/css/CSSAnimations.h (right):

https://codereview.chromium.org/1120003002/diff/320001/Source/core/animation/...
Source/core/animation/css/CSSAnimations.h:82: ~RunningAnimation() { }
Define it out-of-line?

[peria, 2015-06-01 04:43:02]

https://codereview.chromium.org/1120003002/diff/320001/Source/core/animation/...
File Source/core/animation/Animation.h (right):

https://codereview.chromium.org/1120003002/diff/320001/Source/core/animation/...
Source/core/animation/Animation.h:58: USING_PRE_FINALIZER(Animation,
disposeAnimation);
On 2015/05/30 11:34:04, sof wrote:
> No need to add a TODO, but eagerly finalizing Animation objects will be an
> alternative.

Done.

https://codereview.chromium.org/1120003002/diff/320001/Source/core/animation/...
File Source/core/animation/AnimationStack.cpp (left):

https://codereview.chromium.org/1120003002/diff/320001/Source/core/animation/...
Source/core/animation/AnimationStack.cpp:91: // std::sort doesn't work with
OwnPtrs
On 2015/05/30 11:34:04, sof wrote:
> nit: s/OwnPtr/Member/ (or remove the comment, doesn't add much.)

Done.

https://codereview.chromium.org/1120003002/diff/320001/Source/core/animation/...
File Source/core/animation/AnimationTimeline.cpp (right):

https://codereview.chromium.org/1120003002/diff/320001/Source/core/animation/...
Source/core/animation/AnimationTimeline.cpp:138: HeapVector<Member<Animation>>
animations;
On 2015/05/30 11:34:04, sof wrote:
> Use copyToVector() instead?

Done.

https://codereview.chromium.org/1120003002/diff/320001/Source/core/animation/...
File Source/core/animation/KeyframeEffect.h (right):

https://codereview.chromium.org/1120003002/diff/320001/Source/core/animation/...
Source/core/animation/KeyframeEffect.h:67: const EffectModel* model() const {
return m_model.get(); }
On 2015/05/30 11:34:04, sof wrote:
> nit: redundant get()

Done.

https://codereview.chromium.org/1120003002/diff/320001/Source/core/animation/...
File Source/core/animation/LengthBoxStyleInterpolation.cpp (right):

https://codereview.chromium.org/1120003002/diff/320001/Source/core/animation/...
Source/core/animation/LengthBoxStyleInterpolation.cpp:107:
StyleInterpolation::trace(visitor);
On 2015/05/30 11:34:04, sof wrote:
> nit: move this to the end?

Done.

https://codereview.chromium.org/1120003002/diff/320001/Source/core/animation/...
File Source/core/animation/SampledEffect.h (right):

https://codereview.chromium.org/1120003002/diff/320001/Source/core/animation/...
Source/core/animation/SampledEffect.h:29: HeapVector<Member<Interpolation>>*
mutableInterpolations() { return m_interpolations.get(); }
On 2015/05/30 11:34:05, sof wrote:
> get() needed?

Done.

https://codereview.chromium.org/1120003002/diff/320001/Source/core/animation/...
Source/core/animation/SampledEffect.h:29: HeapVector<Member<Interpolation>>*
mutableInterpolations() { return m_interpolations.get(); }
On 2015/05/30 11:34:05, sof wrote:
> get() needed?

not needed.

https://codereview.chromium.org/1120003002/diff/320001/Source/core/animation/...
File Source/core/animation/StringKeyframe.cpp (right):

https://codereview.chromium.org/1120003002/diff/320001/Source/core/animation/...
Source/core/animation/StringKeyframe.cpp:277: break;
On 2015/05/30 11:34:05, sof wrote:
> Could you fix indentation of this "break"?

Thank you for catching up this mistake!

https://codereview.chromium.org/1120003002/diff/320001/Source/core/animation/...
File Source/core/animation/animatable/AnimatableLengthPoint.h (right):

https://codereview.chromium.org/1120003002/diff/320001/Source/core/animation/...
Source/core/animation/animatable/AnimatableLengthPoint.h:45: const
AnimatableValue* x() const { return m_x.get(); }
On 2015/05/30 11:34:05, sof wrote:
> redundant get()s.

Done.

https://codereview.chromium.org/1120003002/diff/320001/Source/core/animation/...
File Source/core/animation/animatable/AnimatableLengthPoint3D.h (right):

https://codereview.chromium.org/1120003002/diff/320001/Source/core/animation/...
Source/core/animation/animatable/AnimatableLengthPoint3D.h:45: const
AnimatableValue* x() const { return m_x.get(); }
On 2015/05/30 11:34:05, sof wrote:
> yet more redundant get()s

Done.

https://codereview.chromium.org/1120003002/diff/320001/Source/core/animation/...
File Source/core/animation/animatable/AnimatableLengthSize.h (right):

https://codereview.chromium.org/1120003002/diff/320001/Source/core/animation/...
Source/core/animation/animatable/AnimatableLengthSize.h:45: const
AnimatableValue* width() const { return m_width.get(); }
On 2015/05/30 11:34:05, sof wrote:
> get() redundant - could you take a pass through the header files touched here
> and remove them where possible?

Done.

https://codereview.chromium.org/1120003002/diff/320001/Source/core/animation/...
File Source/core/animation/css/CSSAnimations.h (right):

https://codereview.chromium.org/1120003002/diff/320001/Source/core/animation/...
Source/core/animation/css/CSSAnimations.h:82: ~RunningAnimation() { }
On 2015/05/30 11:34:05, sof wrote:
> Define it out-of-line?

Done.

[peria, 2015-06-02 02:29:42]

Yuta-san,
Could you concatenate the result of my experiences on Linux?
https://drive.google.com/a/chromium.org/file/d/0B4QUVw-AB8wPZTBqbmlCdXlpbG8/v...

thank you in advance.

[Yuta Kitamura, 2015-06-02 05:51:44]

Sorry for taking long to get back to you,

(I had to deal with some issue with my tool to be able to
 handle null values in the results...)

Here's the results:
https://www.googledrive.com/host/0B6XhrzUCq5lhUjloNWR3b0xxRXM/2015/moveanim.html

I've only skimmed the results, but consistent regressions in
queueing durations seemed worrisome to me.

[peria, 2015-06-03 05:15:11]

On 2015/06/02 05:51:44, Yuta Kitamura wrote:
> Sorry for taking long to get back to you,
> 
> (I had to deal with some issue with my tool to be able to
>  handle null values in the results...)
> 
> Here's the results:
>
https://www.googledrive.com/host/0B6XhrzUCq5lhUjloNWR3b0xxRXM/2015/moveanim.html
> 
> I've only skimmed the results, but consistent regressions in
> queueing durations seemed worrisome to me.

As we discussed offline, we have to measure performance again with
enabling lazy sweep and idle GC.
Here is my experiments' result. Could you concatenate it again?
https://drive.google.com/file/d/0B4QUVw-AB8wPZFYtWVBVZGdLTVk/view?usp=sharing

[Yuta Kitamura, 2015-06-05 05:21:49]

Here's the revised results, along with your data:
https://www.googledrive.com/host/0B6XhrzUCq5lhUjloNWR3b0xxRXM/2015/moveanim2....

[peria, 2015-06-05 06:22:02]

On 2015/06/05 05:21:49, Yuta Kitamura wrote:
> Here's the revised results, along with your data:
>
https://www.googledrive.com/host/0B6XhrzUCq5lhUjloNWR3b0xxRXM/2015/moveanim2....

Thank you very much for the cooperation.

It's interesting that a new metric, which means it was not included in past
report,
"responsive-total_big_jank_thread_time" regresses largely in many pages,
especially on Nexus 7.

[haraken, 2015-06-08 02:15:06]

On 2015/06/05 06:22:02, peria wrote:
> On 2015/06/05 05:21:49, Yuta Kitamura wrote:
> > Here's the revised results, along with your data:
> >
>
https://www.googledrive.com/host/0B6XhrzUCq5lhUjloNWR3b0xxRXM/2015/moveanim2....
> 
> Thank you very much for the cooperation.
> 
> It's interesting that a new metric, which means it was not included in past
> report,
> "responsive-total_big_jank_thread_time" regresses largely in many pages,
> especially on Nexus 7.

Thanks for the results!

- Would you summarize the results?
- Would you start a thread in oilpan-reviews including Ross and Sami and ask
what the responsive-total_big_jank_thread_time is and how we should take care of
the metric?

[peria, 2015-06-08 02:22:05]

On 2015/06/08 02:15:06, haraken wrote:
> 
> Thanks for the results!
> 
> - Would you summarize the results?
I'm writing a doc.
Will share it soon.

> - Would you start a thread in oilpan-reviews including Ross and Sami and ask
> what the responsive-total_big_jank_thread_time is and how we should take care
of
> the metric?
will do.
JFYI, the metric has an explanation in code.
https://code.google.com/p/chromium/codesearch#chromium/src/tools/telemetry/te...

[peria, 2015-09-07 15:10:01]

haraken@ did this work with https://codereview.chromium.org/1318543009/ and
https://codereview.chromium.org/1276183004/ instead.

Let me close this issue.