Issue 10783035: Create frequently used symbols in the vm isolate

Issue 10783035: Create frequently used symbols in the vm isolate (Closed)

Created:
8 years, 5 months ago by siva

Modified:
8 years, 5 months ago

Reviewers:
Ivan Posva

CC:
reviews_dartlang.org, vm-dev_dartlang.org

Base URL:
http://dart.googlecode.com/svn/branches/bleeding_edge/dart/runtime/

Visibility:
Public.

More Reviews

Description

Create frequently used symbols in the vm isolate - Avoids the need for doing a NewSymbol on these everytime - saves space as they get shared by isolates Committed: https://code.google.com/p/dart/source/detail?r=9834

Patch Set 1 #

Patch Set 2 : #

Patch Set 3 : #

Patch Set 4 : #

Total comments: 8

Patch Set 5 : #

Patch Set 6 : #

Total comments: 16

Patch Set 7 : #

Created: 8 years, 5 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+876 lines, -600 lines)			Patch
M	lib/isolate.cc	View	1 2 3 4 5 6	7 chunks	+10 lines, -9 lines	0 comments	Download
M	lib/math.cc	View	1 2 3 4 5 6	2 chunks	+2 lines, -1 line	0 comments	Download
M	lib/string.cc	View	1 2 3 4 5 6	2 chunks	+2 lines, -1 line	0 comments	Download
M	vm/class_finalizer.cc	View	1 2 3 4 5 6	1 chunk	+1 line, -0 lines	0 comments	Download
M	vm/class_finalizer_test.cc	View	1 2 3 4 5 6	3 chunks	+3 lines, -2 lines	0 comments	Download
M	vm/code_descriptors_test.cc	View	1 2 3 4 5 6	3 chunks	+3 lines, -2 lines	0 comments	Download
M	vm/code_generator.cc	View	1 2 3 4 5 6	7 chunks	+7 lines, -6 lines	0 comments	Download
M	vm/code_generator_test.cc	View	1 2 3 4 5 6	8 chunks	+8 lines, -7 lines	0 comments	Download
M	vm/code_patcher_ia32_test.cc	View	1 2 3 4 5 6	3 chunks	+3 lines, -2 lines	0 comments	Download
M	vm/code_patcher_x64_test.cc	View	1 2 3 4 5 6	3 chunks	+3 lines, -2 lines	0 comments	Download
M	vm/compiler.cc	View	1 2 3 4 5 6	2 chunks	+2 lines, -1 line	0 comments	Download
M	vm/compiler_test.cc	View	1 2 3 4 5 6	2 chunks	+2 lines, -1 line	0 comments	Download
M	vm/dart.cc	View	1 2 3 4 5 6	2 chunks	+2 lines, -0 lines	0 comments	Download
M	vm/dart_api_impl.cc	View	1 2 3 4 5 6	6 chunks	+7 lines, -6 lines	0 comments	Download
M	vm/dart_entry.cc	View	1 2 3 4 5 6	7 chunks	+8 lines, -7 lines	0 comments	Download
M	vm/dart_entry_test.cc	View	1 2 3 4 5 6	5 chunks	+5 lines, -4 lines	0 comments	Download
M	vm/debugger.cc	View	1 2 3 4 5 6	2 chunks	+2 lines, -1 line	0 comments	Download
M	vm/debugger_api_impl.cc	View	1 2 3 4 5 6	2 chunks	+2 lines, -1 line	0 comments	Download
M	vm/exceptions.cc	View	1 2 3 4 5 6	4 chunks	+16 lines, -15 lines	0 comments	Download
M	vm/find_code_object_test.cc	View	1 2 3 4 5 6	3 chunks	+3 lines, -2 lines	0 comments	Download
M	vm/flow_graph_builder.cc	View	1 2 3 4 5 6	7 chunks	+7 lines, -6 lines	0 comments	Download
M	vm/flow_graph_compiler.cc	View	1 2 3 4 5 6	2 chunks	+2 lines, -1 line	0 comments	Download
M	vm/flow_graph_compiler_ia32.cc	View	1 2 3 4 5 6	2 chunks	+2 lines, -1 line	0 comments	Download
M	vm/flow_graph_compiler_x64.cc	View	1 2 3 4 5 6	2 chunks	+2 lines, -1 line	0 comments	Download
M	vm/intermediate_language.cc	View	1 2 3 4 5 6	2 chunks	+3 lines, -2 lines	0 comments	Download
M	vm/intermediate_language_ia32.cc	View	1 2 3 4 5 6	5 chunks	+5 lines, -4 lines	0 comments	Download
M	vm/intermediate_language_x64.cc	View	1 2 3 4 5 6	5 chunks	+5 lines, -4 lines	0 comments	Download
M	vm/intrinsifier_ia32.cc	View	1 2 3 4 5 6	3 chunks	+4 lines, -3 lines	0 comments	Download
M	vm/intrinsifier_x64.cc	View	1 2 3 4 5 6	2 chunks	+3 lines, -2 lines	0 comments	Download
M	vm/isolate.h	View	1 2 3 4 5 6	2 chunks	+8 lines, -8 lines	0 comments	Download
M	vm/isolate.cc	View	1 2 3 4 5 6	1 chunk	+3 lines, -1 line	0 comments	Download
M	vm/object.h	View	1 2 3 4 5 6	4 chunks	+4 lines, -9 lines	0 comments	Download
M	vm/object.cc	View	1 2 3 4 5 6	115 chunks	+236 lines, -350 lines	0 comments	Download
M	vm/object_test.cc	View	1 2 3 4 5 6	16 chunks	+57 lines, -56 lines	0 comments	Download
M	vm/parser.cc	View	1 2 3 4 5 6	45 chunks	+56 lines, -55 lines	0 comments	Download
M	vm/parser_test.cc	View	1 2 3 4 5 6	4 chunks	+5 lines, -4 lines	0 comments	Download
M	vm/raw_object_snapshot.cc	View	1 2 3 4 5 6	2 chunks	+2 lines, -1 line	0 comments	Download
M	vm/resolver_test.cc	View	1 2 3 4 5 6	2 chunks	+2 lines, -1 line	0 comments	Download
M	vm/runtime_entry_test.cc	View	1 2 3 4 5 6	2 chunks	+2 lines, -1 line	0 comments	Download
M	vm/scanner.cc	View	1 2 3 4 5 6	8 chunks	+12 lines, -11 lines	0 comments	Download
M	vm/scopes.cc	View	1 2 3 4 5 6	2 chunks	+2 lines, -1 line	0 comments	Download
M	vm/snapshot.cc	View	1 2 3 4 5 6	2 chunks	+2 lines, -2 lines	0 comments	Download
M	vm/snapshot_test.cc	View	1 2 3 4 5 6	2 chunks	+2 lines, -1 line	0 comments	Download
M	vm/stub_code_ia32_test.cc	View	1 2 3 4 5 6	2 chunks	+2 lines, -1 line	0 comments	Download
M	vm/stub_code_x64_test.cc	View	1 2 3 4 5 6	2 chunks	+2 lines, -1 line	0 comments	Download
A	vm/symbols.h	View	1 2 3 4 5 6	1 chunk	+80 lines, -0 lines	0 comments	Download
A	vm/symbols.cc	View	1 2 3 4 5 6	1 chunk	+269 lines, -0 lines	0 comments	Download
M	vm/unit_test.cc	View	1 2 3 4 5 6	4 chunks	+4 lines, -3 lines	0 comments	Download
M	vm/vm_sources.gypi	View	1 2 3 4 5 6	1 chunk	+2 lines, -0 lines	0 comments	Download

Messages

Total messages: 6 (0 generated)

Expand Messages | Collapse Messages

Ivan Posva

https://chromiumcodereview.appspot.com/10783035/diff/14001/vm/object.cc File vm/object.cc (right): https://chromiumcodereview.appspot.com/10783035/diff/14001/vm/object.cc#newcode8786 vm/object.cc:8786: RawString* String::NewSymbol(const T* characters, intptr_t len) { This needs ...

8 years, 5 months ago (2012-07-18 04:09:22 UTC) #3

siva

Addressed review comments, PTAL https://chromiumcodereview.appspot.com/10783035/diff/14001/vm/object.cc File vm/object.cc (right): https://chromiumcodereview.appspot.com/10783035/diff/14001/vm/object.cc#newcode8786 vm/object.cc:8786: RawString* String::NewSymbol(const T* characters, intptr_t ...

8 years, 5 months ago (2012-07-19 01:12:34 UTC) #4

Ivan Posva

LGTM with comments. -Ivan https://chromiumcodereview.appspot.com/10783035/diff/14010/vm/dart_api_impl.cc File vm/dart_api_impl.cc (right): https://chromiumcodereview.appspot.com/10783035/diff/14010/vm/dart_api_impl.cc#newcode2701 vm/dart_api_impl.cc:2701: const String& dot = String::Handle(Symbols::New(".")); ...

8 years, 5 months ago (2012-07-23 18:26:34 UTC) #5

siva

8 years, 5 months ago (2012-07-23 23:09:00 UTC) #6

Addressed comments.

https://chromiumcodereview.appspot.com/10783035/diff/14010/vm/dart_api_impl.cc
File vm/dart_api_impl.cc (right):

https://chromiumcodereview.appspot.com/10783035/diff/14010/vm/dart_api_impl.c...
vm/dart_api_impl.cc:2701: const String& dot = String::Handle(Symbols::New("."));
On 2012/07/23 18:26:34, Ivan Posva wrote:
> Symbols.Dot()?

Done.

https://chromiumcodereview.appspot.com/10783035/diff/14010/vm/dart_api_impl.c...
vm/dart_api_impl.cc:3095: dot_name = Symbols::New(".");
On 2012/07/23 18:26:34, Ivan Posva wrote:
> Symbols.Dot()?

Done.

https://chromiumcodereview.appspot.com/10783035/diff/14010/vm/dart_api_impl.c...
vm/dart_api_impl.cc:3097: const String& dot = String::Handle(isolate,
Symbols::New("."));
Fixed this and checked for other spots too.

On 2012/07/23 18:26:34, Ivan Posva wrote:
> Symbols.Dot()?
> 
> Maybe others?

https://chromiumcodereview.appspot.com/10783035/diff/14010/vm/object.cc
File vm/object.cc (left):

https://chromiumcodereview.appspot.com/10783035/diff/14010/vm/object.cc#oldco...
vm/object.cc:6962: const Class& context_class =
Class::Handle(Object::context_class());
On 2012/07/23 18:26:34, Ivan Posva wrote:
> No assertion here? Maybe others...

Done.

https://chromiumcodereview.appspot.com/10783035/diff/14010/vm/object.cc
File vm/object.cc (right):

https://chromiumcodereview.appspot.com/10783035/diff/14010/vm/object.cc#newco...
vm/object.cc:313: Object::Allocate(cls.id(), Instance::InstanceSize(),
Heap::kOld);
On 2012/07/23 18:26:34, Ivan Posva wrote:
> Why are you even setting up cls here? Can't you just use the null class id
> constant directly?

Done.

https://chromiumcodereview.appspot.com/10783035/diff/14010/vm/symbols.cc
File vm/symbols.cc (right):

https://chromiumcodereview.appspot.com/10783035/diff/14010/vm/symbols.cc#newc...
vm/symbols.cc:20: void Symbols::Init(Isolate* isolate) {
On 2012/07/23 18:26:34, Ivan Posva wrote:
> Maybe name this InitOnce to make it clearer?

Done.

https://chromiumcodereview.appspot.com/10783035/diff/14010/vm/symbols.cc#newc...
vm/symbols.cc:24: // Pre-allocate the Array and OneByteString class in the vm
isolate so that
On 2012/07/23 18:26:34, Ivan Posva wrote:
> How about turning this around? These two need to be present before calling
> Symbols::Init?

Done.

https://chromiumcodereview.appspot.com/10783035/diff/14010/vm/symbols.h
File vm/symbols.h (right):

https://chromiumcodereview.appspot.com/10783035/diff/14010/vm/symbols.h#newco...
vm/symbols.h:46: // Add string into the symbol table.
On 2012/07/23 18:26:34, Ivan Posva wrote:
> // Add the string into the VM isolate symbol table.

Done.

Expand Messages | Collapse Messages