Object pool with support for untagged entries.

Object pool with support for untagged entries.

This adds meta-information to object pool entries to allow storing
untagged immediates or code addresses (ExternalLabel) directly.

This eliminates the need to generate extra code to preserve the LSB
when storing immediates as smis (x64, arm64).

BUG=

Committed: https://github.com/dart-lang/sdk/commit/265a54411a0b949e5577610b15cec084e3ea49cc

[Florian Schneider, 2015-06-09 13:18:42]

fschneider@google.com changed reviewers:
+ iposva@google.com, srdjan@google.com

[Florian Schneider, 2015-06-09 13:18:43]

This is a different approach than https://codereview.chromium.org/1147303002/.
Complexity-wise this CL is much simpler.

The issue with the other CL (sorting the entries, and fixing up the generated
code after wards using AssemblerFixups) is that it requires to generate a larger
code pattern for loading from the pool on arm and mips - the final constant pool
index is not known until after code generation.

It comes at the cost of more expensive scanning of pointers in the pool.

[Florian Schneider, 2015-06-09 13:23:08]

https://codereview.chromium.org/1175523002/diff/20001/runtime/vm/raw_object.h
File runtime/vm/raw_object.h (right):

https://codereview.chromium.org/1175523002/diff/20001/runtime/vm/raw_object.h...
runtime/vm/raw_object.h:1010: RawTypedData* info_array_;
This int8-array holds the entry-type for each entry in the object pool.

[srdjan, 2015-06-09 21:49:47]

LGTMwC

Do you have any data about memory footprint changes?

https://codereview.chromium.org/1175523002/diff/40001/runtime/vm/assembler_ar...
File runtime/vm/assembler_arm.cc (right):

https://codereview.chromium.org/1175523002/diff/40001/runtime/vm/assembler_ar...
runtime/vm/assembler_arm.cc:2665: FindExternalLabel(label, kPatchable));
Should fit on two instead of three lines.

https://codereview.chromium.org/1175523002/diff/40001/runtime/vm/assembler_arm.h
File runtime/vm/assembler_arm.h (right):

https://codereview.chromium.org/1175523002/diff/40001/runtime/vm/assembler_ar...
runtime/vm/assembler_arm.h:344: ObjectPoolHelper& object_pool() { return
object_pool_; }
Should you rename object_pool_ and related, since it is actually not the
object_pool that you are returning?
ObjectPoolHelper is slightly clumsy name, IMO. Maybe ObjectPoolWrapper?

https://codereview.chromium.org/1175523002/diff/40001/runtime/vm/assembler_x6...
File runtime/vm/assembler_x64.cc (right):

https://codereview.chromium.org/1175523002/diff/40001/runtime/vm/assembler_x6...
runtime/vm/assembler_x64.cc:36: object_pool_.AddObject(Object::null_object());
On one line?

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

https://codereview.chromium.org/1175523002/diff/40001/runtime/vm/object.h#new...
runtime/vm/object.h:3506: class ObjectPool : public Object {
Add a brief comment above.

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

https://codereview.chromium.org/1175523002/diff/40001/runtime/vm/raw_object.c...
runtime/vm/raw_object.cc:537: intptr_t len = raw_obj->ptr()->length_;
const

[Florian Schneider, 2015-06-10 08:26:18]

A quick measurement on dart2js memory usage shows a 300K increase for object
pool. This can be reduced by a compressing the info array (from now 1 byte per
entry to 2 bits per entry) with that I guess the additional memory will be <100K
for dart2js.

https://codereview.chromium.org/1175523002/diff/40001/runtime/vm/assembler_ar...
File runtime/vm/assembler_arm.cc (right):

https://codereview.chromium.org/1175523002/diff/40001/runtime/vm/assembler_ar...
runtime/vm/assembler_arm.cc:2665: FindExternalLabel(label, kPatchable));
On 2015/06/09 21:49:46, srdjan wrote:
> Should fit on two instead of three lines.

Done.

https://codereview.chromium.org/1175523002/diff/40001/runtime/vm/assembler_arm.h
File runtime/vm/assembler_arm.h (right):

https://codereview.chromium.org/1175523002/diff/40001/runtime/vm/assembler_ar...
runtime/vm/assembler_arm.h:344: ObjectPoolHelper& object_pool() { return
object_pool_; }
On 2015/06/09 21:49:46, srdjan wrote:
> Should you rename object_pool_ and related, since it is actually not the
> object_pool that you are returning?
> ObjectPoolHelper is slightly clumsy name, IMO. Maybe ObjectPoolWrapper?

Done.

https://codereview.chromium.org/1175523002/diff/40001/runtime/vm/assembler_x6...
File runtime/vm/assembler_x64.cc (right):

https://codereview.chromium.org/1175523002/diff/40001/runtime/vm/assembler_x6...
runtime/vm/assembler_x64.cc:36: object_pool_.AddObject(Object::null_object());
On 2015/06/09 21:49:47, srdjan wrote:
> On one line?

Done.

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

https://codereview.chromium.org/1175523002/diff/40001/runtime/vm/object.h#new...
runtime/vm/object.h:3506: class ObjectPool : public Object {
On 2015/06/09 21:49:47, srdjan wrote:
> Add a brief comment above.

Done.

[Florian Schneider, 2015-06-10 09:41:28]

Committed patchset #5 (id:80001) manually as
265a54411a0b949e5577610b15cec084e3ea49cc.