VM: Refactor allocation stats code and remove duplicate code.

VM: Refactor allocation stats code and remove duplicate code.

Make context allocation stub shared between isolates.

The fast-path used in optimized code still has isolate-specific
addresses inlined for performance reasons.

Common functions are moved into class_table.h/.cc.

This is a preparation for adding an isolate-independent version of the
allocation code and allocation tracking code.

BUG=
R=johnmccutchan@google.com

Committed: https://github.com/dart-lang/sdk/commit/0cf617ef1392d206b2f5484927de29d541b13119

[Florian Schneider, 2015-07-15 10:45:02]

fschneider@google.com changed reviewers:
+ johnmccutchan@google.com

[Florian Schneider, 2015-07-15 12:25:37]

Hold off with reviewing this for now until I upload a new version.

[Florian Schneider, 2015-07-16 13:33:00]

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

[Florian Schneider, 2015-07-16 13:33:01]

PTAL.

This includes https://codereview.chromium.org/1235433004/, which is necessary to
make the allocation code shareable between isolates without too many
indirections.

[koda, 2015-07-16 15:45:18]

DBC for future thought

https://chromiumcodereview.appspot.com/1241863002/diff/20001/runtime/vm/assem...
File runtime/vm/assembler_arm.cc (right):

https://chromiumcodereview.appspot.com/1241863002/diff/20001/runtime/vm/assem...
runtime/vm/assembler_arm.cc:3363: LoadIsolate(dest);
In this branch, one must not use "class_table", nor "table_ptr", so it would be
more robust if those locals were not even visible here.

In general, how fast/obviously will we fail if someone by mistake inlines
isolate-specific things during precompilation?

I think we should consider funneling these "reinterpret_cast" through some
interface that can assert that we're not precompiling.

[Cutch, 2015-07-16 15:47:23]

LGTM with some comments:

It would be nice to have a flag controlling the global default of inline_isolate
in the allocation tracing code. This way we can easily test that both code paths
work for all classes.

[Florian Schneider, 2015-07-17 07:36:45]

On 2015/07/16 15:47:23, Cutch wrote:
> LGTM with some comments:
> 
> It would be nice to have a flag controlling the global default of
inline_isolate
> in the allocation tracing code. This way we can easily test that both code
paths
> work for all classes.

I thought about a flag in the assembler, but then maybe we want inlining of
these constants in optimized code, but not in shared code.

[Florian Schneider, 2015-07-17 07:40:40]

https://chromiumcodereview.appspot.com/1241863002/diff/20001/runtime/vm/assem...
File runtime/vm/assembler_arm.cc (right):

https://chromiumcodereview.appspot.com/1241863002/diff/20001/runtime/vm/assem...
runtime/vm/assembler_arm.cc:3363: LoadIsolate(dest);
On 2015/07/16 15:45:18, koda wrote:
> In this branch, one must not use "class_table", nor "table_ptr", so it would
be
> more robust if those locals were not even visible here.
> 
> In general, how fast/obviously will we fail if someone by mistake inlines
> isolate-specific things during precompilation?
> 
> I think we should consider funneling these "reinterpret_cast" through some
> interface that can assert that we're not precompiling.

Ok, I'll try to rearrange the code accordingly by checking

if (inline_isolate) first.


Often you just get a segfault. We have to check all places where we do something
like Immediate(reinterpret_cast<uword|int32_t|int64_t>some_ptr)

[Florian Schneider, 2015-07-17 13:15:39]

Committed patchset #3 (id:40001) manually as
0cf617ef1392d206b2f5484927de29d541b13119 (presubmit successful).

[koda, 2015-07-17 13:20:24]

On 2015/07/17 07:40:40, Florian Schneider wrote:
>
https://chromiumcodereview.appspot.com/1241863002/diff/20001/runtime/vm/assem...
> File runtime/vm/assembler_arm.cc (right):
> 
>
https://chromiumcodereview.appspot.com/1241863002/diff/20001/runtime/vm/assem...
> runtime/vm/assembler_arm.cc:3363: LoadIsolate(dest);
> On 2015/07/16 15:45:18, koda wrote:
> > In this branch, one must not use "class_table", nor "table_ptr", so it would
> be
> > more robust if those locals were not even visible here.
> > 
> > In general, how fast/obviously will we fail if someone by mistake inlines
> > isolate-specific things during precompilation?
> > 
> > I think we should consider funneling these "reinterpret_cast" through some
> > interface that can assert that we're not precompiling.
> 
> Ok, I'll try to rearrange the code accordingly by checking
> 
> if (inline_isolate) first.
> 
> 
> Often you just get a segfault. We have to check all places where we do
something
> like Immediate(reinterpret_cast<uword|int32_t|int64_t>some_ptr)

Yes, we have quite a few of those naked reinterpret_casts. Ideally, we should
have both:
1. a single interface for embedding C++ pointers as immediates (which will check
any flag/mode)
2. a paranoid assert that looks for any immediate that is within the .text
segment of the running process without having gone through the above interface