Reimplement Symbols using hash table template.

Reimplement Symbols using hash table template.

In the process, add hash table support for materializing an Object key from a lookup key on demand.

Also use this to simplify CompressedTokenStream.

R=iposva@google.com

Committed: https://code.google.com/p/dart/source/detail?r=38710

[Ivan Posva, 2014-07-23 12:09:26]

Thanks,
-Ivan

https://codereview.chromium.org/397413002/diff/80001/runtime/vm/symbols.cc
File runtime/vm/symbols.cc (right):

https://codereview.chromium.org/397413002/diff/80001/runtime/vm/symbols.cc#ne...
runtime/vm/symbols.cc:178: template<typename CharType>
As below I would like to discuss this in person as I have a hard time following
through all of the different layers here.

https://codereview.chromium.org/397413002/diff/80001/runtime/vm/symbols.cc#ne...
runtime/vm/symbols.cc:236: isolate->object_store()->set_symbol_table(array);
I find this pattern really awkward. Reading just this code here you are storing
back the original value into the isolate's object store.

Let's look at alternative ways of dealing with this kind of code in person.

[Ivan Posva, 2014-07-29 21:55:10]

LGTMwC -ip

https://codereview.chromium.org/397413002/diff/80001/runtime/vm/symbols.cc
File runtime/vm/symbols.cc (right):

https://codereview.chromium.org/397413002/diff/80001/runtime/vm/symbols.cc#ne...
runtime/vm/symbols.cc:141: StringSlice(const String* str, intptr_t begin_index,
intptr_t length)
Why do you use "const String* str" instead of a reference?

https://codereview.chromium.org/397413002/diff/80001/runtime/vm/symbols.cc#ne...
runtime/vm/symbols.cc:283: template<typename StringType>
Please add a comment enumerating all possible StringTypes coming in here. I
think this can be easily confused with the different String types from
object.h/rawobject.h

[koda, 2014-07-29 22:31:50]

https://codereview.chromium.org/397413002/diff/80001/runtime/vm/symbols.cc
File runtime/vm/symbols.cc (right):

https://codereview.chromium.org/397413002/diff/80001/runtime/vm/symbols.cc#ne...
runtime/vm/symbols.cc:141: StringSlice(const String* str, intptr_t begin_index,
intptr_t length)
On 2014/07/29 21:55:10, Ivan Posva wrote:
> Why do you use "const String* str" instead of a reference?

A remnant from when this was just a struct. Changed to ref.

https://codereview.chromium.org/397413002/diff/80001/runtime/vm/symbols.cc#ne...
runtime/vm/symbols.cc:178: template<typename CharType>
On 2014/07/23 12:09:26, Ivan Posva wrote:
> As below I would like to discuss this in person as I have a hard time
following
> through all of the different layers here.

Acknowledged.

https://codereview.chromium.org/397413002/diff/80001/runtime/vm/symbols.cc#ne...
runtime/vm/symbols.cc:236: isolate->object_store()->set_symbol_table(array);
On 2014/07/23 12:09:26, Ivan Posva wrote:
> I find this pattern really awkward. Reading just this code here you are
storing
> back the original value into the isolate's object store.
> 
> Let's look at alternative ways of dealing with this kind of code in person.

Acknowledged.

https://codereview.chromium.org/397413002/diff/80001/runtime/vm/symbols.cc#ne...
runtime/vm/symbols.cc:283: template<typename StringType>
On 2014/07/29 21:55:10, Ivan Posva wrote:
> Please add a comment enumerating all possible StringTypes coming in here. I
> think this can be easily confused with the different String types from
> object.h/rawobject.h

Done.

[koda, 2014-07-29 22:48:03]

Committed patchset #5 manually as r38710 (presubmit successful).