Tree-shaking: use a hash set for tracking live selectors, drop uncompiled functions.

Tree-shaking: use a hash set for tracking live selectors; drop uncompiled functions.

hello_world 8.8 -> 7.4MB heap

R=srdjan@google.com

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

[rmacnak, 2015-08-05 18:11:54]

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

[rmacnak, 2015-08-05 18:11:55]

https://codereview.chromium.org/1275653002/diff/1/runtime/vm/precompiler.cc
File runtime/vm/precompiler.cc (left):

https://codereview.chromium.org/1275653002/diff/1/runtime/vm/precompiler.cc#o...
runtime/vm/precompiler.cc:484: // TODO(rmacnak): Do not create the symbol if it
does not already exist.
Turns out the vast majority of these selectors already have both the getter name
and simple name in the symbol table, so adding code to lookup an existing symbol
didn't save much.

[srdjan, 2015-08-05 18:20:08]

lgtm

https://codereview.chromium.org/1275653002/diff/1/runtime/vm/hash_map.h
File runtime/vm/hash_map.h (right):

https://codereview.chromium.org/1275653002/diff/1/runtime/vm/hash_map.h#newcode8
runtime/vm/hash_map.h:8: #include "vm/zone.h"
Why is this include needed?

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

https://codereview.chromium.org/1275653002/diff/1/runtime/vm/precompiler.cc#n...
runtime/vm/precompiler.cc:87: dropped_function_count_);
ISL_Print

https://codereview.chromium.org/1275653002/diff/1/runtime/vm/precompiler.cc#n...
runtime/vm/precompiler.cc:614: OS::Print("Precompilation dropping %s\n",
ISL_Print

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

https://codereview.chromium.org/1275653002/diff/1/runtime/vm/precompiler.h#ne...
runtime/vm/precompiler.h:23: class SymbolPair {
: public ValueObject ?

[Cutch, 2015-08-05 18:25:04]

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

[Cutch, 2015-08-05 18:25:05]

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

https://codereview.chromium.org/1275653002/diff/1/runtime/vm/precompiler.cc#n...
runtime/vm/precompiler.cc:87: dropped_function_count_);
On 2015/08/05 18:20:08, srdjan wrote:
> ISL_Print

DBC: You may also want to put a:

LogBlock lb(Thread::Current());

At the top of ::DoCompileAll so that the output all precompiler traces are
output in a block.

[rmacnak, 2015-08-05 19:51:51]

https://codereview.chromium.org/1275653002/diff/1/runtime/vm/hash_map.h
File runtime/vm/hash_map.h (right):

https://codereview.chromium.org/1275653002/diff/1/runtime/vm/hash_map.h#newcode8
runtime/vm/hash_map.h:8: #include "vm/zone.h"
On 2015/08/05 18:20:08, srdjan wrote:
> Why is this include needed?

Zone allocation happens in this file. The other includes of the file just happen
to include zone.h themselves.

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

https://codereview.chromium.org/1275653002/diff/1/runtime/vm/precompiler.cc#n...
runtime/vm/precompiler.cc:87: dropped_function_count_);
On 2015/08/05 18:25:05, Cutch wrote:
> On 2015/08/05 18:20:08, srdjan wrote:
> > ISL_Print
> 
> DBC: You may also want to put a:
> 
> LogBlock lb(Thread::Current());
> 
> At the top of ::DoCompileAll so that the output all precompiler traces are
> output in a block.

Done.

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

https://codereview.chromium.org/1275653002/diff/1/runtime/vm/precompiler.h#ne...
runtime/vm/precompiler.h:23: class SymbolPair {
On 2015/08/05 18:20:08, srdjan wrote:
> : public ValueObject ?

DirectChainedHashMap uses the assignment operator.

[rmacnak, 2015-08-05 20:21:46]

Committed patchset #2 (id:20001) manually as
edf288280e2b2213974bdc1d816e785a0815d134 (presubmit successful).

[Florian Schneider, 2015-08-06 08:21:19]

dbc:

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

https://codereview.chromium.org/1275653002/diff/20001/runtime/vm/precompiler....
runtime/vm/precompiler.h:27: typedef bool Value;
I think you can do something simpler:

typedef String* Key;
typedef String* Value;
typedef String* Pair;

See PointerLeyValueTrait in hash_map.h.

NULL means non-existent in this case.