Dedup stackmaps.

Canonicalize stackmaps after precompilation.

precompiled.isolate -4.3% dart2js ARM

R=fschneider@google.com

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

[rmacnak, 2015-10-06 01:03:52]

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

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

LGTM.

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

https://codereview.chromium.org/1387613003/diff/40001/runtime/vm/precompiler....
runtime/vm/precompiler.cc:772: Library& lib = Library::Handle(Z);
This code for iterating over all functions seems to be used in multiple places.
Maybe factor it out into Function-Iterator?

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

https://codereview.chromium.org/1387613003/diff/40001/runtime/vm/precompiler....
runtime/vm/precompiler.h:49: Key key_;
No need for key_ either.

This is basicaly the same as PointerKeyValueTrait - modulo IsKeyEqual, which
invokes Equals, instead of comparing the raw-ptr. Actually, PointerKeyValueTrait
may just work as a drop-in replacement.

https://codereview.chromium.org/1387613003/diff/40001/runtime/vm/precompiler....
runtime/vm/precompiler.h:97: static inline intptr_t Hashcode(Key key) {
Can you avoid this class and use PointerKeyValueTrait instead, if you add a
corresponding Hashcode method to Stackmap?

https://codereview.chromium.org/1387613003/diff/40001/runtime/vm/precompiler....
runtime/vm/precompiler.h:110: class StackmapSet : public ValueObject {
typedef DirectChainedHashmap<PointerKeyValueTrait<Stackmap> > StackmapSet;

[rmacnak, 2015-10-07 22:19:06]

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

https://codereview.chromium.org/1387613003/diff/40001/runtime/vm/precompiler....
runtime/vm/precompiler.cc:772: Library& lib = Library::Handle(Z);
On 2015/10/07 10:40:07, Florian Schneider wrote:
> This code for iterating over all functions seems to be used in multiple
places.
> Maybe factor it out into Function-Iterator?

Recast as visitor. Covers clear code, bind static calls and dedup stackmaps, but
shrinking the function tables and finding new dynamic functions still have
custom iteration.

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

https://codereview.chromium.org/1387613003/diff/40001/runtime/vm/precompiler....
runtime/vm/precompiler.h:49: Key key_;
On 2015/10/07 10:40:07, Florian Schneider wrote:
> No need for key_ either.
> 

Oh, I misunderstood this to be used as the backing storage of the map.

> This is basicaly the same as PointerKeyValueTrait - modulo IsKeyEqual, which
> invokes Equals, instead of comparing the raw-ptr. Actually,
PointerKeyValueTrait
> may just work as a drop-in replacement.

https://codereview.chromium.org/1387613003/diff/40001/runtime/vm/precompiler....
runtime/vm/precompiler.h:97: static inline intptr_t Hashcode(Key key) {
On 2015/10/07 10:40:07, Florian Schneider wrote:
> Can you avoid this class and use PointerKeyValueTrait instead, if you add a
> corresponding Hashcode method to Stackmap?

Not quite. It wants Stackmap::Equals(Stackmap*&) instead of
Stackmap::Equals(Stackmap&)

https://codereview.chromium.org/1387613003/diff/40001/runtime/vm/precompiler....
runtime/vm/precompiler.h:110: class StackmapSet : public ValueObject {
On 2015/10/07 10:40:07, Florian Schneider wrote:
> typedef DirectChainedHashmap<PointerKeyValueTrait<Stackmap> > StackmapSet;

Dropped StackmapSet and SymbolsSet for typedefs.

[rmacnak, 2015-10-07 23:22:07]

Committed patchset #4 (id:60001) manually as
d8189f90c6eafddd0ca153e70d747f37c7590968 (presubmit successful).