Fix to allocation stub invalidation: we cannot just remove it as it will be collected even though c…

Fix to allocation stub invalidation: we cannot just remove it as it will be collected even though code still may reference it. Since we should probably have two different allocation stubs, keep two in the class and switch between them. Future work will put intelligence in the allocation stub selection. Added Daniel's test flag & code.

R=koda@google.com

Committed: https://code.google.com/p/dart/source/detail?r=40327

[srdjan, 2014-09-12 18:21:52]

srdjan@google.com changed reviewers:
+ koda@google.com

[koda, 2014-09-15 14:59:13]

This lgtm as a fix for the immediate bug, although we should talk about the
longer-term plan.

https://codereview.chromium.org/566853005/diff/40001/runtime/tests/vm/dart/by...
File runtime/tests/vm/dart/byte_array_test.dart (right):

https://codereview.chromium.org/566853005/diff/40001/runtime/tests/vm/dart/by...
runtime/tests/vm/dart/byte_array_test.dart:2384: for (var i=0; i<20; i++) {
This test might be substantially slower now; if you think that could be a
problem, one easy way to speed it up would be to remove/shorten this loop (not
sure why we repeat the test like this).

https://codereview.chromium.org/566853005/diff/40001/runtime/vm/object.cc
File runtime/vm/object.cc (right):

https://codereview.chromium.org/566853005/diff/40001/runtime/vm/object.cc#new...
runtime/vm/object.cc:3638: void Class::DisableAllocationStub() const {
Since you are changing the behavior, please add/update the specification in the
header. Perhaps rename it "SwitchAllocationStub".

[srdjan, 2014-09-15 23:39:59]

Long term plan is to use similar method as done for static methods: add
references to stub code in the compiled code, replace/remove as the stub code
gets disabled so that stub-code gets GC-d when nobody refers to it any longer.

https://codereview.chromium.org/566853005/diff/40001/runtime/tests/vm/dart/by...
File runtime/tests/vm/dart/byte_array_test.dart (right):

https://codereview.chromium.org/566853005/diff/40001/runtime/tests/vm/dart/by...
runtime/tests/vm/dart/byte_array_test.dart:2384: for (var i=0; i<20; i++) {
On 2014/09/15 14:59:12, koda wrote:
> This test might be substantially slower now; if you think that could be a
> problem, one easy way to speed it up would be to remove/shorten this loop (not
> sure why we repeat the test like this).

Slow down is not measurable in debug mode. The invalidation and regeneration
overhead is apparently negligible compared to the cost of e.g., GC code in debug
mode.

https://codereview.chromium.org/566853005/diff/40001/runtime/vm/object.cc
File runtime/vm/object.cc (right):

https://codereview.chromium.org/566853005/diff/40001/runtime/vm/object.cc#new...
runtime/vm/object.cc:3638: void Class::DisableAllocationStub() const {
On 2014/09/15 14:59:12, koda wrote:
> Since you are changing the behavior, please add/update the specification in
the
> header. Perhaps rename it "SwitchAllocationStub".

Renamed to SwitchAllocationStub.

[srdjan, 2014-09-16 17:01:24]

Committed patchset #7 (id:120001) manually as r40327 (presubmit successful).