Load runtime entries from the Thread instead of the ObjectPool.

Load runtime entries from the Thread instead of the ObjectPool.

Removes the need to relocate their ObjectPool entries when loading a precompiled snapshot.

Saves about ~2kB of ObjectPool in hello_world at the cost of ~400B/Thread (64-bit).

R=fschneider@google.com

Committed: https://github.com/dart-lang/sdk/commit/9d8dcd1288127b78621d5bf0391de554fe56510f

[rmacnak, 2015-08-30 23:08:50]

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

[Florian Schneider, 2015-08-31 06:51:57]

Why do you want to avoid relocating those entries? It seems easier to to just
that. Or could we do a similar approach as for native functions?

https://chromiumcodereview.appspot.com/1315893004/diff/60001/runtime/vm/runti...
File runtime/vm/runtime_entry_mips.cc (right):

https://chromiumcodereview.appspot.com/1315893004/diff/60001/runtime/vm/runti...
runtime/vm/runtime_entry_mips.cc:50: __ lw(T9, Address(THR,
Thread::OffsetFromThread(this)));
The offset may exceed the maximum that fits into the Address operand.

[rmacnak, 2015-08-31 18:24:24]

https://chromiumcodereview.appspot.com/1315893004/diff/60001/runtime/vm/runti...
File runtime/vm/runtime_entry_mips.cc (right):

https://chromiumcodereview.appspot.com/1315893004/diff/60001/runtime/vm/runti...
runtime/vm/runtime_entry_mips.cc:50: __ lw(T9, Address(THR,
Thread::OffsetFromThread(this)));
On 2015/08/31 06:51:56, Florian Schneider wrote:
> The offset may exceed the maximum that fits into the Address operand. 

This CL brings the 32-bit sizeof(Thread) to 276, versus the 4096/65536 we can
address with a simple load-word in ARM/MIPS.

[rmacnak, 2015-08-31 18:29:21]

On 2015/08/31 06:51:57, Florian Schneider wrote:
> Why do you want to avoid relocating those entries? It seems easier to to just
> that. Or could we do a similar approach as for native functions?

It's nice to not have to do relocation at all.

[Florian Schneider, 2015-09-01 06:19:05]

https://chromiumcodereview.appspot.com/1315893004/diff/60001/runtime/vm/runti...
File runtime/vm/runtime_entry.h (left):

https://chromiumcodereview.appspot.com/1315893004/diff/60001/runtime/vm/runti...
runtime/vm/runtime_entry.h:21: #define RUNTIME_ENTRY_LIST(V)                    
                             \
Why move this to a separate file?

https://chromiumcodereview.appspot.com/1315893004/diff/60001/runtime/vm/runti...
File runtime/vm/runtime_entry_list.h (right):

https://chromiumcodereview.appspot.com/1315893004/diff/60001/runtime/vm/runti...
runtime/vm/runtime_entry_list.h:46: V(TestSmiSub)                               
                                \
We don't want to have test function stored in each thread object.

https://chromiumcodereview.appspot.com/1315893004/diff/60001/runtime/vm/runti...
runtime/vm/runtime_entry_list.h:64: V(RawObject*, TestLeafSmiAdd, RawObject*,
RawObject*)                        \
Same here.

https://chromiumcodereview.appspot.com/1315893004/diff/60001/runtime/vm/runti...
File runtime/vm/runtime_entry_mips.cc (right):

https://chromiumcodereview.appspot.com/1315893004/diff/60001/runtime/vm/runti...
runtime/vm/runtime_entry_mips.cc:50: __ lw(T9, Address(THR,
Thread::OffsetFromThread(this)));
On 2015/08/31 18:24:24, rmacnak wrote:
> On 2015/08/31 06:51:56, Florian Schneider wrote:
> > The offset may exceed the maximum that fits into the Address operand. 
> 
> This CL brings the 32-bit sizeof(Thread) to 276, versus the 4096/65536 we can
> address with a simple load-word in ARM/MIPS.

Ok, seems there is room. Please make sure that there are assertions in place
about the max offset.

https://chromiumcodereview.appspot.com/1315893004/diff/60001/runtime/vm/runti...
File runtime/vm/runtime_entry_test.cc (left):

https://chromiumcodereview.appspot.com/1315893004/diff/60001/runtime/vm/runti...
runtime/vm/runtime_entry_test.cc:42: DEFINE_RUNTIME_ENTRY(TestSmiSub, 2) {
Can the test runtime functions stay in the _test.cc file?

[Florian Schneider, 2015-09-01 10:44:02]

LGTM with comments addressed.

[rmacnak, 2015-09-01 17:26:54]

https://chromiumcodereview.appspot.com/1315893004/diff/60001/runtime/vm/runti...
File runtime/vm/runtime_entry.h (left):

https://chromiumcodereview.appspot.com/1315893004/diff/60001/runtime/vm/runti...
runtime/vm/runtime_entry.h:21: #define RUNTIME_ENTRY_LIST(V)                    
                             \
On 2015/09/01 06:19:05, Florian Schneider wrote:
> Why move this to a separate file?

Breaking an include cycle (allocation, thread, runtime_entry).

https://chromiumcodereview.appspot.com/1315893004/diff/60001/runtime/vm/runti...
File runtime/vm/runtime_entry_list.h (right):

https://chromiumcodereview.appspot.com/1315893004/diff/60001/runtime/vm/runti...
runtime/vm/runtime_entry_list.h:46: V(TestSmiSub)                               
                                \
On 2015/09/01 06:19:05, Florian Schneider wrote:
> We don't want to have test function stored in each thread object.

True, but if they aren't there we need a special case in emitting runtime calls
that doesn't load from the thread and we are no longer testing the same
mechanism. Or we need a mechanism to only include the test functions in these
lists when building run_vm_tests.

[Florian Schneider, 2015-09-02 08:22:03]

https://chromiumcodereview.appspot.com/1315893004/diff/60001/runtime/vm/runti...
File runtime/vm/runtime_entry.h (left):

https://chromiumcodereview.appspot.com/1315893004/diff/60001/runtime/vm/runti...
runtime/vm/runtime_entry.h:21: #define RUNTIME_ENTRY_LIST(V)                    
                             \
On 2015/09/01 17:26:54, rmacnak wrote:
> On 2015/09/01 06:19:05, Florian Schneider wrote:
> > Why move this to a separate file?
> 
> Breaking an include cycle (allocation, thread, runtime_entry).

I see - fine as a last resort if there is no way of breaking this cycle by
introducing forward declarations instead of including.

https://chromiumcodereview.appspot.com/1315893004/diff/60001/runtime/vm/runti...
File runtime/vm/runtime_entry_list.h (right):

https://chromiumcodereview.appspot.com/1315893004/diff/60001/runtime/vm/runti...
runtime/vm/runtime_entry_list.h:46: V(TestSmiSub)                               
                                \
On 2015/09/01 17:26:54, rmacnak wrote:
> On 2015/09/01 06:19:05, Florian Schneider wrote:
> > We don't want to have test function stored in each thread object.
> 
> True, but if they aren't there we need a special case in emitting runtime
calls
> that doesn't load from the thread and we are no longer testing the same
> mechanism. Or we need a mechanism to only include the test functions in these
> lists when building run_vm_tests.

Maybe we can just test runtime invocation using the existing functions and
remove those completely then? It's not a lot of space, but seems like a bad
smell, when we have unused slots for test functions here in a release build.

e.g. invoke AllocateArray or LibcPow instead of calling these test-only
functions.

[rmacnak, 2015-09-02 21:58:18]

https://codereview.chromium.org/1315893004/diff/60001/runtime/vm/runtime_entr...
File runtime/vm/runtime_entry_list.h (right):

https://codereview.chromium.org/1315893004/diff/60001/runtime/vm/runtime_entr...
runtime/vm/runtime_entry_list.h:46: V(TestSmiSub)                               
                                \
On 2015/09/02 08:22:03, Florian Schneider wrote:
> On 2015/09/01 17:26:54, rmacnak wrote:
> > On 2015/09/01 06:19:05, Florian Schneider wrote:
> > > We don't want to have test function stored in each thread object.
> > 
> > True, but if they aren't there we need a special case in emitting runtime
> calls
> > that doesn't load from the thread and we are no longer testing the same
> > mechanism. Or we need a mechanism to only include the test functions in
these
> > lists when building run_vm_tests.
> 
> Maybe we can just test runtime invocation using the existing functions and
> remove those completely then? It's not a lot of space, but seems like a bad
> smell, when we have unused slots for test functions here in a release build.
> 
> e.g. invoke AllocateArray or LibcPow instead of calling these test-only
> functions.

Done.

[rmacnak, 2015-09-02 21:58:32]

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