Shared token objects

Shared token objects

Instead of one array for literal and identifier tokens for each TokenStream, share one array among all TokenStreams in an isolate.

The shared array and the token-to-index map is stored in the object store until the embedder is done loading. Then the map is thrown away (and the array remains shared, referred to by the TokenStreams).

When new files get loaded, a new shared array is created. For temporary scripts, e.g. when evaluating one-shot functions, the token arrays are not shared.

This eliminates many duplicate LiteralToken and identifier tokens. Looking at the core libraries:

Before: 30877 identifiers and 9618 literal tokens
After: 12899 identifiers and 6371 literal tokens

BUG=
R=asiva@google.com

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

[hausner, 2015-10-27 22:26:20]

Description was changed from

==========
Shared token objects

BUG=
==========

to

==========
Shared token objects

Instead of one array for literal and identifier tokens for each TokenStream,
share one array among all TokenStreams in an isolate. 

The shared array and the token-to-index map is stored in the object store until
the embedder is done loading. Then the map is thrown away (and the array remains
shared, referred to by the TokenStreams).

When new files get loaded, a new shared array is created. For temporary scripts,
e.g. when evaluating one-shot functions, the token arrays are not shared.

This eliminates many duplicate LiteralToken and identifier tokens. Looking at
the core libraries:

Before: 30877 identifiers and 9618 literal tokens
After: 12899 identifiers and 6371 literal tokens

BUG=
==========

[hausner, 2015-10-27 22:26:20]

hausner@google.com changed reviewers:
+ iposva@google.com

[hausner, 2015-10-27 22:26:34]

hausner@google.com changed reviewers:
+ asiva@google.com

[siva, 2015-10-28 18:30:23]

LGTM.

https://chromiumcodereview.appspot.com/1420103006/diff/20001/runtime/vm/objec...
File runtime/vm/object.cc (right):

https://chromiumcodereview.appspot.com/1420103006/diff/20001/runtime/vm/objec...
runtime/vm/object.cc:8358: static const bool kPrintTokenObjects = false;
Instead of a compile time option to toggle this tracing do you think it makes
sense to add a trace_tokenization flag.

https://chromiumcodereview.appspot.com/1420103006/diff/20001/runtime/vm/objec...
runtime/vm/object.cc:8391: Object::Handle(tokens_->GetOrNull(descriptor,
&is_present));
You could create these handles (Object, LiteralToken, Smi::Handle etc.) in the
constructor of CompressedTokenStreamData as AddLiteralToken, AddIdentToken etc.
are called in a loop.

https://chromiumcodereview.appspot.com/1420103006/diff/20001/runtime/vm/objec...
runtime/vm/object.cc:8528:
isolate->object_store()->set_token_objects_map(Array::Handle());
set_token_objects_map(null_array());

Not sure if we discussed storing the backing data array as the token_objects_
array at this point in order to avoid the extra indirection during parsing when
CurrentToken is accessed.

https://chromiumcodereview.appspot.com/1420103006/diff/20001/runtime/vm/object.h
File runtime/vm/object.h (right):

https://chromiumcodereview.appspot.com/1420103006/diff/20001/runtime/vm/objec...
runtime/vm/object.h:3126: bool used_shared_tokens);
use_shared_tokens

[hausner, 2015-10-28 21:28:46]

Thank you. Running 

dart --trace_isolates tests/language/hello_dart_test.dart

I get the following measurements:

vm isolate snapshot size:
before: 943k
after: 918k
Reduction of about 25k

Heap:
before: New space (0k of 0k) Old space (774k of 1156k)
after: New space (0k of 0k) Old space (706k of 1156k)
Reduction of heap about 68k

https://chromiumcodereview.appspot.com/1420103006/diff/20001/runtime/vm/objec...
File runtime/vm/object.cc (right):

https://chromiumcodereview.appspot.com/1420103006/diff/20001/runtime/vm/objec...
runtime/vm/object.cc:8358: static const bool kPrintTokenObjects = false;
On 2015/10/28 18:30:22, siva wrote:
> Instead of a compile time option to toggle this tracing do you think it makes
> sense to add a trace_tokenization flag.

I expect this to go away soon, so I didn't want to expose it to the command
line.

https://chromiumcodereview.appspot.com/1420103006/diff/20001/runtime/vm/objec...
runtime/vm/object.cc:8391: Object::Handle(tokens_->GetOrNull(descriptor,
&is_present));
On 2015/10/28 18:30:22, siva wrote:
> You could create these handles (Object, LiteralToken, Smi::Handle etc.) in the
> constructor of CompressedTokenStreamData as AddLiteralToken, AddIdentToken
etc.
> are called in a loop.

Done.

https://chromiumcodereview.appspot.com/1420103006/diff/20001/runtime/vm/objec...
runtime/vm/object.cc:8528:
isolate->object_store()->set_token_objects_map(Array::Handle());
On 2015/10/28 18:30:22, siva wrote:
> set_token_objects_map(null_array());
> 
> Not sure if we discussed storing the backing data array as the token_objects_
> array at this point in order to avoid the extra indirection during parsing
when
> CurrentToken is accessed.

We did discuss it. I'll add this to the TokenIterator class in a separate
checkin.

https://chromiumcodereview.appspot.com/1420103006/diff/20001/runtime/vm/object.h
File runtime/vm/object.h (right):

https://chromiumcodereview.appspot.com/1420103006/diff/20001/runtime/vm/objec...
runtime/vm/object.h:3126: bool used_shared_tokens);
On 2015/10/28 18:30:22, siva wrote:
> use_shared_tokens

Doned.

[hausner, 2015-10-28 23:03:10]

Committed patchset #3 (id:40001) manually as
59dce43dfebd40b5881c8c1ae3dd30622897f0f8 (presubmit successful).