PSA: Enable Oilpan on ToT

[Kentaro Hara]

Hi

I discussed with release managers and we have decided to enable Oilpan on ToT. I'm planning to make the first attempt on Jan 25 JST.

Our plan is:

- Enable Oilpan on ToT on all platforms (Win/Mac/Linux/Android/ChromeOS).

- If any unacceptable stability issue is observed, revert Oilpan immediately.

- Oilpan is going to make dramatic changes to performance/memory results. Overall Oilpan will improve performance but may regress a couple of specific benchmarks. Even if some unexpected performance/memory regressions are observed, we'll keep Oilpan on ToT for a week and then make a final judgement.

- If we're super lucky, we can just keep Oilpan on ToT, send it to Dev, and forever :)

- I'll update the shipping status on this thread.

If you have any concern or question, please let me know.

<details>

We have already shipped Oilpan to Win/Mac Canaries and fixed all crash reports but haven't yet shipped it to Android/ChromeOS Canaries. At first, we were planning to ship it to Android/ChromeOS Canaries and see how it goes, but it turned out that the users of Android/ChromeOS Canaries are limited and we can't get a meaningful number of crash reports there. So we've decided to skip the process and enable it on ToT.

</details>

--

Kentaro Hara, Tokyo, Japan

[Elliott Sprehn]

SGTM, good luck!

[Dimitri Glazkov]

\o/

:DG<

[Chris Harrelson]

Hi Kentaro,

Will the oilpan bots start blocking the CQ at the same time?

--
You received this message because you are subscribed to the Google Groups "blink-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to blink-dev+...@chromium.org.

[Kentaro Hara]

Will the oilpan bots start blocking the CQ at the same time?

When we enable Oilpan on ToT, the current non-oilpan bots become "oilpan" bots. So yes, they start blocking the CQ (in that sense).

[Chris Harrelson]

On Fri, Jan 22, 2016 at 5:42 PM Kentaro Hara <har...@chromium.org> wrote:

Will the oilpan bots start blocking the CQ at the same time?

When we enable Oilpan on ToT, the current non-oilpan bots become "oilpan" bots. So yes, they start blocking the CQ (in that sense).

Great. I wonder if it's worth it to have explicit non-Oilpan bots in that case. Otherwise it seems likely the build will break by turning off Oilpan. Is that planned?

Chris

[Kentaro Hara]

On Fri, Jan 22, 2016 at 5:55 PM, Chris Harrelson <chri...@chromium.org> wrote:

On Fri, Jan 22, 2016 at 5:42 PM Kentaro Hara <har...@chromium.org> wrote:

Will the oilpan bots start blocking the CQ at the same time?

When we enable Oilpan on ToT, the current non-oilpan bots become "oilpan" bots. So yes, they start blocking the CQ (in that sense).

Great. I wonder if it's worth it to have explicit non-Oilpan bots in that case. Otherwise it seems likely the build will break by turning off Oilpan. Is that planned?

It's a substantial amount of work on the infra team, so not planned.

The confusion you're worried about will happen if we have to repeat the flag flip a couple of times. Our plan is to finish shipping Oilpan at the first attempt (or at the second attempt in the worst case) and thus reduces the pain. That's why we've spent a lot of time stabilizing the Oilpan infrastructure until we receive 0 crash reports from Win/Mac Canaries :)

[Elliott Sprehn]

Can't we just swap the oilpan bot to build without the define set? That turns it into the non-oilpan bot at the same time we turn all other bots into oilpan bots. :)

[Kentaro Hara]

+dpranke

One thing we could do is:

- Land enable_oilpan=1 on ToT. At this point, the current non-oilpan bots become "oilpan" bots.

- At the same time, change the recipes of the current oilpan bots to use enable_oilpan=0. Then the current oilpan bots become "non-oilpan" bots.

We can try that if the infra team is okay with the idea. (I'm a bit afraid that this will cause an even worse confusion on developers though :-)

[Dirk Pranke]

We could do that if so desired.

Personally I think that's being too conservative. 

It doesn't seem very likely to me that we're both going to find an issue w/ oilpan

at this point that is so severe that we can't just fix it instead of turning it off and 

that the non-oilpan build will become so broken w/o bots that we can't fix it if

need be.

So maintaining the non-oilpan build just feels like wasted effort.

But, I don't feel that strongly about it. It won't be much of my effort either way :)

-- Dirk

[Kentaro Hara]

On Mon, Jan 25, 2016 at 1:55 PM, Dirk Pranke <dpr...@chromium.org> wrote:

We could do that if so desired.

Personally I think that's being too conservative. 

It doesn't seem very likely to me that we're both going to find an issue w/ oilpan

at this point that is so severe that we can't just fix it instead of turning it off and 

that the non-oilpan build will become so broken w/o bots that we can't fix it if

need be.

So maintaining the non-oilpan build just feels like wasted effort.

Yeah. I also found that it's not that trivial to turn the oilpan bots into non-oilpan bots because currently we don't have a way to specify NonOilpanTestExpectations.

Let me just enable Oilpan on ToT (without having non-oilpan bots) and try our best to fix incoming issues asap. If that gets out of our control, I'll revert Oilpan and consider a more conservative way to enable it.

[Dirk Pranke]

On Sun, Jan 24, 2016 at 9:02 PM, Kentaro Hara <har...@chromium.org> wrote:

On Mon, Jan 25, 2016 at 1:55 PM, Dirk Pranke <dpr...@chromium.org> wrote:

We could do that if so desired.

Personally I think that's being too conservative. 

It doesn't seem very likely to me that we're both going to find an issue w/ oilpan

at this point that is so severe that we can't just fix it instead of turning it off and 

that the non-oilpan build will become so broken w/o bots that we can't fix it if

need be.

So maintaining the non-oilpan build just feels like wasted effort.

Yeah. I also found that it's not that trivial to turn the oilpan bots into non-oilpan bots because currently we don't have a way to specify NonOilpanTestExpectations.

Can't you just re-use the existing OilpanTestExpections file (apart from the potential confusion of the name, which we can always rename if we need to)?

-- Dirk

[Kentaro Hara]

On Mon, Jan 25, 2016 at 2:12 PM, Dirk Pranke <dpr...@chromium.org> wrote:

On Sun, Jan 24, 2016 at 9:02 PM, Kentaro Hara <har...@chromium.org> wrote:

On Mon, Jan 25, 2016 at 1:55 PM, Dirk Pranke <dpr...@chromium.org> wrote:

We could do that if so desired.

Personally I think that's being too conservative. 

It doesn't seem very likely to me that we're both going to find an issue w/ oilpan

at this point that is so severe that we can't just fix it instead of turning it off and 

that the non-oilpan build will become so broken w/o bots that we can't fix it if

need be.

So maintaining the non-oilpan build just feels like wasted effort.

Yeah. I also found that it's not that trivial to turn the oilpan bots into non-oilpan bots because currently we don't have a way to specify NonOilpanTestExpectations.

Can't you just re-use the existing OilpanTestExpections file (apart from the potential confusion of the name, which we can always rename if we need to)?

Ah, that's doable. We could:

1) Turn the oilpan bots into non-oilpan bots.

2) Describe non-oilpan-only failures in OilpanTestExpectations.

(But given the complexity, let me take the approach as the last resort :-)

[Elliott Sprehn]

What tests have different output in oilpan and why?

[Kentaro Hara]

See OilpanExpectations and their associated bugs:

https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit/LayoutTests/OilpanExpectations&q=oilpanexpectations&sq=package:chromium&type=cs

Note that most of the tests become more correct with Oilpan. For example, in the non-oilpan world, Node::m_parentNode can't be a strong reference because it creates a reference cycle. So the test result of fast/dom/gc-dom-tree-lifetime.html in non-oilpan is wrong. Oilpan makes it correct.

[Kentaro Hara]

We enabled Oilpan on ToT at r371208.

If you see any issue about it, ping oilpan-...@chromium.org. A tracking bug is here. I'll keep updating the status on this thread.

[Annie Sullivan]

FYI, the perf dashboard is starting to see the effects of enabling oilpan. You can see all the changes it detected with that CL in the revision range here:

https://chromeperf.appspot.com/group_report?rev=371208

Caveats:

• Other cls in the revision range may have caused some of the changes.
• Some benchmarks are noisy.

[Kentaro Hara]

Thanks Annie!

Here is a quick status update:

1) Enable Oilpan on ToT (<== Done)

2) Stabilize all build bots (<== Done)

3) Push Oilpan to Canaries (<== Done with 50.0.2631.0)

4) Fix all crash reports (<== We're now here)

5) Analyze the performance dashboard

6) Push Oilpan to Devs

At the moment, we've received no crash reports caused by enabling Oilpan.

[Simon Pick]

Wow! That's a lot of green :)

[Alex Clarke]

Overall its looking like a net win, although there appear to be some regressions: https://code.google.com/p/chromium/issues/detail?id=581068

[Kentaro Hara]

On Wed, Jan 27, 2016 at 12:31 AM, Alex Clarke <alexc...@google.com> wrote:

Overall its looking like a net win, although there appear to be some regressions: https://code.google.com/p/chromium/issues/detail?id=581068

Thanks for reporting!

I'm now focusing on fixing incoming crash reports and stabilize. Then I'll take a close look at the performance dashboard -- I'm not planning to ignore the regressions :D

[Kentaro Hara]

Summary: I think we have succeeded in landing Oilpan. I'm planning to keep it on trunk forever.

Details:

- We landed Oilpan at r371208 one week ago.

- Oilpan has caused a bunch of crash reports and we've landed fixes to put out the fire. As far as I compare the top 100 crashers in 50.0.{2631,2632,2633,2634,2635,2636}.0 (Oilpan Canary) and 50.0.2633.3 (Oilpan Dev) with the top 100 crashers in 50.0.2630.0 (Non-Oilpan Canary), I think we have already finished fixing most of the Oilpan-related crashes. The rate of Oilpan-related crashes is now pretty low.

- Some layout tests are failing (see the first section of TestExpectations) but most of them are failing because either 1) Oilpan started providing a more correct lifetime and thus we should just update the test results or 2) tests are timing out in Debug builds because of excessive asserts in Oilpan's infrastructure. keishi@ is now working on removing all the Oilpan-specific failures.

- Oilpan has dramatically changed the performance/memory shape. Overall it was a clear win (an amazingly clear win!) but Oilpan has also caused a bunch of regressions we shouldn't ignore. keishi@, yutak@ and haraken@ are now working on fixing/explaining the following regressions. Our plan is to keep Oilpan on trunk and fix/explain the regressions in follow-up patches.

* memory.memory_health_plan

* v8.key_mobile_sites_smooth

* v8.infinite_scroll

* v8.top_25_smooth

* smoothness.tough_canvas_cases

* smoothness.gpu_rasterization_and_decoding.image_decoding_cases

* smoothness.gpu_rasterization.key_mobile_sites_smooth

* smoothness.gpu_rasterization.tough_filters_cases

* smoothness.fling.simple_mobile_sites

* smoothness.gpu_rasterization.top_25_smooth

Note that I've marked regressions of vm_final_renderer_* as ignored in this graph. This is because the value of vm_final_renderer_* totally depends on the timing when the last GC runs and thus doesn't make much sense. As long as the regression is <10% and there is no regression in the peak memory usage, I think it would be okay to ignore. (vm_final_renderer_* is not a good metric in that sense; we've ignored the regressions many times so far.)

- If there is no objection in this thread, I'm going to remove all WillBe types from the code base. Removing WillBe types will make it harder to merge changes to previous branches (where Oilpan is disabled), so I'll chat with release managers about the best schedule.

Thanks!

[Jochen Eisinger]

congratulations!

The overall plan sounds good!

Could you create a list of work items that could be done by others? I think there are things that can be parallelized to shorten the transition period, everybody on the team should jump in and help.

Similarly, I'd delay cleaning up WillBe types until after all remaining issues with the transition are fixed.

best

-jochen

[Kentaro Hara]

Could you create a list of work items that could be done by others? I think there are things that can be parallelized to shorten the transition period, everybody on the team should jump in and help.

The list is exactly the ones I wrote in the above email.

That said, I'm not sure how much I can distribute the work to other teams. For example, would it be fair to ask the V8 team to fix the regressions in v8 benchmarks, ask the Android Chrome team to fix the regression in memory_health_plan etc? The tricky part is that most of the performance/memory regressions are caused by the parameter tuning or algorithm in Oilpan's infrastructure.

On the other hand, I'd like to distribute the work to clean up the code base to volunteers (e.g., RawPtr<T> => T*, removing unnecessary .get(), removing protecting on-stack pointers etc).

Similarly, I'd delay cleaning up WillBe types until after all remaining issues with the transition are fixed.

 

This is another tricky part. I guess it will take two months to fix/explain all regressions. If we delay the clean-up until then, we need to keep the unused (and even not tested or built) non-Oilpan code for a while.

[Jochen Eisinger]

On Mon, Feb 1, 2016 at 9:45 AM Kentaro Hara <har...@chromium.org> wrote:

Could you create a list of work items that could be done by others? I think there are things that can be parallelized to shorten the transition period, everybody on the team should jump in and help.

The list is exactly the ones I wrote in the above email.

That said, I'm not sure how much I can distribute the work to other teams. For example, would it be fair to ask the V8 team to fix the regressions in v8 benchmarks, ask the Android Chrome team to fix the regression in memory_health_plan etc? The tricky part is that most of the performance/memory regressions are caused by the parameter tuning or algorithm in Oilpan's infrastructure.

On the other hand, I'd like to distribute the work to clean up the code base to volunteers (e.g., RawPtr<T> => T*, removing unnecessary .get(), removing protecting on-stack pointers etc).

Similarly, I'd delay cleaning up WillBe types until after all remaining issues with the transition are fixed.

 

This is another tricky part. I guess it will take two months to fix/explain all regressions. If we delay the clean-up until then, we need to keep the unused (and even not tested or built) non-Oilpan code for a while.

As long as it doesn't take time from those who fix the regressions. 

[Kentaro Hara]

On Mon, Feb 1, 2016 at 6:00 PM, Jochen Eisinger <joc...@chromium.org> wrote:

On Mon, Feb 1, 2016 at 9:45 AM Kentaro Hara <har...@chromium.org> wrote:

Could you create a list of work items that could be done by others? I think there are things that can be parallelized to shorten the transition period, everybody on the team should jump in and help.

The list is exactly the ones I wrote in the above email.

That said, I'm not sure how much I can distribute the work to other teams. For example, would it be fair to ask the V8 team to fix the regressions in v8 benchmarks, ask the Android Chrome team to fix the regression in memory_health_plan etc? The tricky part is that most of the performance/memory regressions are caused by the parameter tuning or algorithm in Oilpan's infrastructure.

On the other hand, I'd like to distribute the work to clean up the code base to volunteers (e.g., RawPtr<T> => T*, removing unnecessary .get(), removing protecting on-stack pointers etc).

Similarly, I'd delay cleaning up WillBe types until after all remaining issues with the transition are fixed.

 

This is another tricky part. I guess it will take two months to fix/explain all regressions. If we delay the clean-up until then, we need to keep the unused (and even not tested or built) non-Oilpan code for a while.

As long as it doesn't take time from those who fix the regressions. 

So I think there are two approaches we can take here.

[Approach 1]

1) Revert Oilpan.

2) Fix the regressions (1 - 2 months).

3) Reland Oilpan.

4) Happy!

[Approach 2]

1) Keep Oilpan as is.

2) Fix the regressions (1 - 2 months).

3) Happy!

[Approach 1] would be safer in a sense that it won't land any regression on trunk. That said, given that Oilpan's overall performance was a significant win and that reverting Oilpan will cause a lot of disturbance & complexity, I'd prefer taking [Approach 2] if you're okay with it.

[Jochen Eisinger]

yes, I think approach 2 is the way to go.

What I'm saying is that everybody should help to finish step 2) of approach 2) as fast as possible :)

[Kentaro Hara]

On Mon, Feb 1, 2016 at 6:13 PM, Jochen Eisinger <joc...@chromium.org> wrote:

yes, I think approach 2 is the way to go.

What I'm saying is that everybody should help to finish step 2) of approach 2) as fast as possible :)

Sure, I'll get more folks involved and finish it asap :)

[Daniel Bratell]

On Mon, 01 Feb 2016 09:17:15 +0100, Kentaro Hara <har...@chromium.org> wrote:

Summary: I think we have succeeded in landing Oilpan. I'm planning to keep it on trunk forever.

Details:

- We landed Oilpan at r371208 one week ago.

Yay!

- Oilpan has dramatically changed the performance/memory shape.

It seems to me that there is a pattern in the tests that you list as greatly improved and the tests that report some problems. The tests that are greatly improved are mostly micro benchmarks while the tests that report some problems are overall high level tests. I had rather seen it being the other way around. :-(

/Daniel

--

/* Opera Software, Linköping, Sweden: CET (UTC+1) */

[Kentaro Hara]

We're fixing them right now :) We've already found ways to fix regressions in v8.* and memory.memory_health_plan.

FWIW, most high-level benchmarks didn't improve or regress with Oilpan. For example, the performance of Speedometer didn't change. The memory consumption of memory.top_7_stress didn't change. Most of the smoothness.* benchmarks didn't change.

 

/Daniel

--

/* Opera Software, Linköping, Sweden: CET (UTC+1) */

[Pavel Feldman]

Similarly, I'd delay cleaning up WillBe types until after all remaining issues with the transition are fixed.

+1, can we get the date for removal announced in advance?

Now is the perfect time for subsystem owners to look carefully at how their code was mapped to Oilpan. In many cases we were barely involved as Oilpan was managing more and more objects.

This is our last chance to look at it and fix/deoilpanize bits of it with just removing the sufix. We won't have the handy declarative ownership markup once prefixes are gone.

[Steve Kobes]

Note that delaying the cleanup prolongs the risk of breaking the non-Oilpan build (since the CQ doesn't test this).  I was bitten by this yesterday in http://crrev.com/1649813003.

[Kentaro Hara]

On Wed, Feb 3, 2016 at 3:48 AM, Pavel Feldman <pfel...@chromium.org> wrote:

Similarly, I'd delay cleaning up WillBe types until after all remaining issues with the transition are fixed.

+1, can we get the date for removal announced in advance?

Sure! We won't make any change without announcing here.

 

Now is the perfect time for subsystem owners to look carefully at how their code was mapped to Oilpan. In many cases we were barely involved as Oilpan was managing more and more objects.

This is our last chance to look at it and fix/deoilpanize bits of it with just removing the sufix. We won't have the handy declarative ownership markup once prefixes are gone.

I agree with your point, but on the other hand, I'm concerned about the risk of keeping the non-Oilpan code since it's even not built or tested right now.

So I'd like to ask each owner to look at it now and fix/deoilpanize bits if needed so that we can remove the non-Oilpan code in two weeks (for example).

Regarding the ownership issue, I'm aware of it -- Oilpan should have a syntax to clarify object ownership. We're discussing that topic in this thread.

[Kentaro Hara]

If there is any concern, I'm planning to remove all WillBe types on Feb 12. If you have any concern, please let me know.

(Until then the Oilpan team will keep non-Oilpan builds buildable.)

[Kentaro Hara]

If there is any concern, I'm planning to remove all WillBe types on Feb 12. If you have any concern, please let me know.

I'm planning to remove 99% of the WillBe types in the code base tomorrow (with this giant CL). If you have any concern, please let me know.

[Emil A Eklund]

On Thu, Feb 11, 2016 at 12:10 AM, Kentaro Hara <har...@chromium.org> wrote:
>> If there is any concern, I'm planning to remove all WillBe types on Feb
>> 12. If you have any concern, please let me know.

This seems _very_ premature. In the past we've always waited until a
new feature has reached stable before removing the old code. Oilpan
hasn't even reached beta yet. What's the rush? Waiting until it has
shipped costs us nothing and gives us options if problems crop up
between now and April.

[Elliott Sprehn]

Without test coverage or bots the non-oilpan code is just rotting away. Are we running the layout tests in non-oilpan mode on some bot?

[Dirk Pranke]

No, I don't think we made that change.

-- Dirk

[Kentaro Hara]

Emil: Are you okay with making the change now?

[Kentaro Hara]

Just to clarify:

Currently no try/build bots are testing non-Oilpan code (Sigbjorn just keeps non-Oilpan code buildable). So what we could do here is:

(Option 1) Remove all non-Oilpan code asap.

(Option 2) Set up non-Oilpan try/build bots. This is an amount of work.

Given that Oilpan has already been stabilized very well in terms of crash reports, performance and memory, it's less likely that we'll need to revert Oilpan in the future. So I'd like to take the Option 1.

(If you don't have an objection, I'd like to land the CL in this weekend. It takes a couple of hours to rebase & upload the CL -- so I want to land it while people are taking a rest :-)

[Emil A Eklund]

On Thu, Feb 11, 2016 at 11:59 PM, Kentaro Hara <har...@chromium.org> wrote:
> Just to clarify:
>
> Currently no try/build bots are testing non-Oilpan code (Sigbjorn just keeps
> non-Oilpan code buildable). So what we could do here is:
>
> (Option 1) Remove all non-Oilpan code asap.
>
> (Option 2) Set up non-Oilpan try/build bots. This is an amount of work.
>
> Given that Oilpan has already been stabilized very well in terms of crash
> reports, performance and memory, it's less likely that we'll need to revert
> Oilpan in the future. So I'd like to take the Option 1.

How can you say that? It's hasn't even reached beta yet and the canary
users are not a representative subset of our install base.

> (If you don't have an objection, I'd like to land the CL in this weekend. It
> takes a couple of hours to rebase & upload the CL -- so I want to land it
> while people are taking a rest :-)

We've had many examples of major issues surfacing late in the process,
the latest that comes to mind being the change to use complex text by
default causing a release-blocking memory regression on android that
was only discovered six weeks after the change has been made.

> Emil: Are you okay with making the change now?

Nothing has changed and I still think this is premature.

Deciding to remove all code for non-opilpan before the first oilpan
release has even reached beta seems very risky to me. We already have
the infrastructure in place to support both, why not repurpose one of
the old oilpan builders as a non-oilpan one and keep it at least until
it's in beta?

If we decide to go ahead anyway I'd at least like to see a plan for
how a major issue would be dealt with.

[Stefan Zager]

On Fri, Feb 12, 2016 at 9:14 AM Emil A Eklund <e...@chromium.org> wrote:

 We already have
the infrastructure in place to support both, why not repurpose one of
the old oilpan builders as a non-oilpan one and keep it at least until
it's in beta?

Without wading into the pro's and con's, I want to say as an ex-infra person that I don't believe this should be overly difficult to do. 

[Dirk Pranke]

I'm pretty sure this is a two line change (if not one line). 

Whether both configurations still work, fixing the non-oilpan one if it doesn't, and still supporting both, of course, is a different story.

-- Dirk

[Sigbjorn Finne]

Den 2/12/2016 18:14, Emil A Eklund skreiv:
> On Thu, Feb 11, 2016 at 11:59 PM, Kentaro Hara <har...@chromium.org> wrote:
>> Just to clarify:
>>
>> Currently no try/build bots are testing non-Oilpan code (Sigbjorn just keeps
>> non-Oilpan code buildable). So what we could do here is:
>>
>> (Option 1) Remove all non-Oilpan code asap.
>>
>> (Option 2) Set up non-Oilpan try/build bots. This is an amount of work.
>>
>> Given that Oilpan has already been stabilized very well in terms of crash
>> reports, performance and memory, it's less likely that we'll need to revert
>> Oilpan in the future. So I'd like to take the Option 1.
>
> How can you say that? It's hasn't even reached beta yet and the canary
> users are not a representative subset of our install base.
>

Oilpan M50s are available on all beta channels.

--sigbjorn

[Sigbjorn Finne]

eh, Friday evening :) s/beta/dev/

--sigbjorn

[Sigbjorn Finne]

Quite, how/who would keep a non-Oilpan configuration in a working state?

My impression so far (having checked&fixed status somewhat regularly
over the past couple of weeks) is that compilation breaks every 2-3 days
atm. I haven't run any tests.

--sigbjorn

[Dominic Cooney]

To summarize, I think we're trying to evaluate this tradeoff:

Cost of continuing to develop with WillBe types +

cost of setting up non-Oilpan bots

versus

probability there are unforseen problems with Oilpan &times;

cost of remediation--for example, reintroducing WillBe types

While the probability of unforseen problems may be low, the cost of reintroducing WillBe types is probably pretty high?

We're still dealing with elevated instability in canary and dev (ping me for a link--Google internal only unfortunately.) Where are with changing from breakpad to crashpad and the changes to throttling from that? Maybe it pays to be conservative at the moment.  My $0.02 is we should probably wait a while to remove the WillBe types. We've lived with them for so long. We can live with them for a few more months.

TL;DR Emil is right.

Dominic

[Pavel Feldman]

Tracing shows 200ms in blink_gc on sfgate.com load. Do we have enough real world performance data with Oilpan enabled that would suggest that it is not a regression on the average case?  Just adding it to the set of potential risks.

[Kentaro Hara]

OK, it looks like we want to keep WillBe types a bit more.

Regarding the cost of keeping WillBe types, my main concern is the cost of making the bots green. Non-oilpan code has not been tested for 3 weeks in any platform (*) and we've landed a couple of complex CLs in the period. It will be an amount of work to make the bots green. I think there are two options:

a) Keep the WillBe types. Set up the non-oilpan bots. Fix all test failures and keep the non-oilpan bots green. 

b) Keep the WillBe types. Set up the non-oilpan bots. Keep the non-oilpan bots pass compilation but leave test failures as is until we decide to revert oilpan from trunk.

Would the option b) be acceptable?

(*) Disclaimer: This topic was already discussed (in this thread) when enabling Oilpan on ToT and there was no objection about it. That's why we didn't introduce/maintain the non-oilpan bots.

[Kentaro Hara]

On Sat, Feb 13, 2016 at 11:51 AM, Pavel Feldman <pfel...@chromium.org> wrote:

Tracing shows 200ms in blink_gc on sfgate.com load. Do we have enough real world performance data with Oilpan enabled that would suggest that it is not a regression on the average case?  Just adding it to the set of potential risks.

This (Google-internal) is UMA from 50.0.2645.0 on Android.

- 95%-tile pause time for Oilpan's GC is 48 ms.

- 95%-tile pause time for V8's minor GC is 24 ms.

- 95%-tile pause time for V8's major GC is 633 ms.

FWIW, we haven't observed any regression in the frame_times metrics in any telemetry benchmarks on Android.

We'll investigate sfgate.com separately.

[Dimitri Glazkov]

On Fri, Feb 12, 2016 at 9:13 PM, Kentaro Hara <har...@chromium.org> wrote:

We'll investigate sfgate.com separately.

Sanity-checking Oilpan by tracing a few known-bad sites seems like a great idea. Given that the stable has non-Oilpan, and the dev channel has Oilpan, it also seems like something even the non-Oilpan folks could chip in with. Let us know if you want the grand Oilpan tracing party organized :)

:DG<

[Chris Harrelson]

On Fri, Feb 12, 2016 at 9:03 PM Kentaro Hara <har...@chromium.org> wrote:

OK, it looks like we want to keep WillBe types a bit more.

Regarding the cost of keeping WillBe types, my main concern is the cost of making the bots green. Non-oilpan code has not been tested for 3 weeks in any platform (*) and we've landed a couple of complex CLs in the period. It will be an amount of work to make the bots green. I think there are two options:

a) Keep the WillBe types. Set up the non-oilpan bots. Fix all test failures and keep the non-oilpan bots green. 

b) Keep the WillBe types. Set up the non-oilpan bots. Keep the non-oilpan bots pass compilation but leave test failures as is until we decide to revert oilpan from trunk.

Would the option b) be acceptable?

Could you first find out how many tests are failing? These could be farmed out to the teams to fix. We could all pitch in to help if needed.

In general, I do think it I'm convinced it is worth maintaining non-Oilpan until we are sure it will stick, even though I think it will be very unlikely to have to be turned off. Annoying I know, but the right software engineering practice for a big change like this.

[Kentaro Hara]

On Wed, Feb 17, 2016 at 4:03 AM, Chris Harrelson <chri...@chromium.org> wrote:

On Fri, Feb 12, 2016 at 9:03 PM Kentaro Hara <har...@chromium.org> wrote:

OK, it looks like we want to keep WillBe types a bit more.

Regarding the cost of keeping WillBe types, my main concern is the cost of making the bots green. Non-oilpan code has not been tested for 3 weeks in any platform (*) and we've landed a couple of complex CLs in the period. It will be an amount of work to make the bots green. I think there are two options:

a) Keep the WillBe types. Set up the non-oilpan bots. Fix all test failures and keep the non-oilpan bots green. 

b) Keep the WillBe types. Set up the non-oilpan bots. Keep the non-oilpan bots pass compilation but leave test failures as is until we decide to revert oilpan from trunk.

Would the option b) be acceptable?

Could you first find out how many tests are failing? These could be farmed out to the teams to fix. We could all pitch in to help if needed.

Yes, I'm now landing CLs to add non-oilpan bots :) Let me see how many tests are failing.

(The challenging part of fixing the failing tests is that we don't have a build history for the past 3 weeks.)

[Kentaro Hara]

(On behalf of Sigbjorn and Keishi, let me write some status update.)

Regarding non-oilpan bots, Sigbjon has fixed all test failures. Now the non-oilpan bots are green. We're planning to keep the non-oilpan bots & WillBe types until Oilpan is shipped to the beta channel and the Blink community agrees on keeping Oilpan on trunk forever.

Regarding the long GC pause time in sfgate.com (reported by pfeldman), Keishi is landing a fix.

[Sigbjorn Finne]

Den 2/18/2016 19:41, Kentaro Hara skreiv:
> (On behalf of Sigbjorn and Keishi, let me write some status update.)
>

> Regarding non-oilpan bots, Sigbjorn has fixed all test failures. Now the
> non-oilpan bots are green
> <https://build.chromium.org/p/chromium.webkit/builders/WebKit%20Linux%20non-Oilpan>.

> We're planning to keep the non-oilpan bots & WillBe types until Oilpan is
> shipped to the beta channel and the Blink community agrees on keeping
> Oilpan on trunk forever.
>

I've only taken care of one block of redness
(https://codereview.chromium.org/1703113003/) , there's plenty more if
you look through all the bot results:

- https://codereview.chromium.org/1709033002/ - yoav is working on
addressing DOMTokenList-induced crashes due to ref-counting details not
quite in place.
- unclassified bunch of imported/csswg-test/css-writing-modes-3 crashes.
- some animations-related instability.

And more. The last two are undiagnosed in the main, could be related to
the first one.

Now that we have set up non-Oilpan bots, please have a look at (flaky)
failures & fix what you can.

--sigbjorn

[Jochen Eisinger]

Now that M50 has branched, and we decided to not revert Oilpan, I think it might be about time to start removing the non-oilpan bots, and the non-oilpan code?

[Kentaro Hara]

M50 went to beta yesterday, so it might be better to wait for a bit more days to collect crash reports from the beta.

At the moment, I don't see any new issues in the crash reports :)

[Michael Hablich]

Clank 50 beta is not yet pushed, so I would propose waiting until this is done.

[Chris Harrelson]

We might as well also wait a week after all beta channels have released, then ping this thread with crash stats before removing the code.

Chris

--

[Sigbjorn Finne]

Den 3/11/2016 17:16, Chris Harrelson skreiv:
> We might as well also wait a week after all beta channels have released,
> then ping this thread with crash stats before removing the code.
>

Makes sense.

The non-Oilpan ToT status is good to great atm, so I'm quite sure we can
handle this dual setup for a while longer without it bringing too much
overhead & trouble for people.

--sigbjorn

[Kentaro Hara]

Some updates about the Oilpan status:

- M50 (including Oilpan) finally went to Android's beta channel today (see omaha proxy). I'll analyze M50's crash reports on all platforms and post the summary to this thread at the end of the next week (around Mar 25). Hopefully we can remove all WillBe types and non-oilpan code at the end of March.

- Until then, we need to keep the "Oilpan" bots green. Remember that the "Oilpan" bots are running non-Oilpan builds. (Sorry for the confusion -- the infra team tried to rename it to "non-Oilpan" but it turned out to cause a bunch of infra issues, so we decided to keep the current name.)

[Sigbjorn Finne]

Den 3/17/2016 11:30, Kentaro Hara skreiv:
> Some updates about the Oilpan status:
>
> - M50 (including Oilpan) finally went to Android's beta channel today
> (see omaha

> proxy <https://omahaproxy.appspot.com/>). I'll analyze M50's crash reports

> on all platforms and post the summary to this thread at the end of the next
> week (around Mar 25). Hopefully we can remove all WillBe types and
> non-oilpan code at the end of March.
>
> - Until then, we need to keep the "Oilpan" bots green. Remember that the
> "Oilpan" bots are running non-Oilpan builds. (Sorry for the confusion --
> the infra team tried to rename it to "non-Oilpan" but it turned out to
> cause a bunch of infra issues, so we decided to keep the current name.)
>

Just to supplement haraken's clarification, this refers to
tryserver.blink trybot naming: foo_oilpan_bar is a bot compiling with
enable_oilpan=0. tryserver.blink bots without a mention of "oilpan" are
Oilpan enabled.

The naming for chromium.webkit bots (
https://build.chromium.org/p/chromium.webkit/builders ) is accurate.

--sigbjorn

[Kentaro Hara]

One week has passed since Oilpan was shipped to beta on all platforms.

The latest status of Oilpan is as follows:

- No release-blocker crash bug is filed relating to Oilpan. I compared the top 200 crash reports (Google-internal) between 50.0.2630.0 (where Oilpan is not enabled) and 50.0.2661.49 (where Oilpan is enabled). I found a couple of Oilpan-related crashes there but none of them is release-blocker bugs (all the crash bugs are tracked in this bug).

- Oilpan-related UMA results on Android are collected in this page (Google-internal). 80% of the marking phases (BlinkGC.CollectGarbage) are within 10 ms. 90% of the marking phases are within 30 ms. 90% of the sweeping phases (BlinkGC.CompleteSweep) are within 12 ms. Overall, pause times caused by Oilpan GCs are about twice as large as pause times caused by V8 minor GCs. I think these numbers are acceptable.

- No real memory regression was observed in the System Health Plan when we shipped Oilpan to Android beta.

In conclusion, it would be fair to say that Oilpan looks good enough in terms of performance, memory and stability.

If there is no objection on this thread, I'm planning to remove all WillBe types next week (around Mar 29).

Thanks!

[Kentaro Hara]

I pushed a CL to remove all WillBe types to CQ.

[Jochen Eisinger]

can we also remove the non-oilpan bots from the main waterfall now?

[Kentaro Hara]

Yes, we're planning to remove non-oilpan bots, !ENABLE(OILPAN) code, RawPtr and a bunch of dead code.

[Rune Lillesveen]

On Wed, Mar 30, 2016 at 5:50 PM, Kentaro Hara <har...@chromium.org> wrote:
> I pushed a CL to remove all WillBe types to CQ.

If we land any new types or variable declarations before your CLs
land, we should now use pure Oilpan types and not the transitional
ones, right?

--
Rune Lillesveen

[Kentaro Hara]

Correct.

Sorry about many confusions -- I failed at landing the 13000 LOC change and decided to remove WillBe types a bit more incrementally.

I've just landed a CL to set ENABLE_OILPAN=1 unconditionally (regardless of the value of enable_oilpan). So we already have no way to run non-oilpan code (i.e., XWillBeY unconditionally behaves as Y). I'm trying to remove all WillBe types as soon as possible.

[Kentaro Hara]

Status update:

- I removed most WillBe types from the code base, but not all. Give me one more day to complete the work.

- You no longer need to use WillBe types at all. Now XWillBeY unconditionally behaves as Y. So you should just use Y.