Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1130)

Issue 10914050: Use external byte arrays for token stream, this moves the token stream out of the isolate heap. For… (Closed)

Created:
8 years, 3 months ago by siva
Modified:
8 years, 3 months ago
Reviewers:
hausner, cshapiro
CC:
reviews_dartlang.org, vm-dev_dartlang.org
Visibility:
Public.

Description

Use external byte arrays for token stream, this moves the token stream out of the isolate heap. For full snapshots the external byte array points to the snapshot buffer thus incurring no extra space in the C heap either. Heap usage stats before this change is: New space (0k of 32768k) Old space (1085k of 1280k) Code space (0k of 0k) Heap Usage stats after this change is: New space (0k of 32768k) Old space (947k of 1024k) Code space (0k of 0k) A savings of about 138k. Committed: https://code.google.com/p/dart/source/detail?r=11977

Patch Set 1 #

Total comments: 4

Patch Set 2 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+130 lines, -89 lines) Patch
M vm/datastream.h View 1 1 chunk +16 lines, -0 lines 0 comments Download
M vm/object.h View 1 5 chunks +14 lines, -23 lines 0 comments Download
M vm/object.cc View 1 10 chunks +46 lines, -42 lines 0 comments Download
M vm/raw_object.h View 1 2 chunks +5 lines, -4 lines 0 comments Download
M vm/raw_object.cc View 1 2 chunks +1 line, -9 lines 0 comments Download
M vm/raw_object_snapshot.cc View 1 2 chunks +9 lines, -7 lines 0 comments Download
M vm/snapshot.h View 1 5 chunks +14 lines, -0 lines 0 comments Download
M vm/snapshot.cc View 1 2 chunks +20 lines, -3 lines 0 comments Download
M vm/snapshot_test.cc View 1 1 chunk +5 lines, -1 line 0 comments Download

Messages

Total messages: 5 (0 generated)
siva
8 years, 3 months ago (2012-09-01 00:07:54 UTC) #1
hausner
drive-by cheer!
8 years, 3 months ago (2012-09-04 17:23:58 UTC) #2
cshapiro
lgtm https://chromiumcodereview.appspot.com/10914050/diff/1/vm/object.cc File vm/object.cc (right): https://chromiumcodereview.appspot.com/10914050/diff/1/vm/object.cc#newcode4798 vm/object.cc:4798: ::free(peer); It is always safe to free NULL. ...
8 years, 3 months ago (2012-09-05 20:33:50 UTC) #3
cshapiro
http://codereview.chromium.org/10914050/diff/1/vm/snapshot.cc File vm/snapshot.cc (right): http://codereview.chromium.org/10914050/diff/1/vm/snapshot.cc#newcode419 vm/snapshot.cc:419: reinterpret_cast<void*>(array), make line 419 a NULL instead of non-NULL
8 years, 3 months ago (2012-09-06 00:11:49 UTC) #4
siva
8 years, 3 months ago (2012-09-06 18:19:29 UTC) #5
https://chromiumcodereview.appspot.com/10914050/diff/1/vm/object.cc
File vm/object.cc (right):

https://chromiumcodereview.appspot.com/10914050/diff/1/vm/object.cc#newcode4798
vm/object.cc:4798: ::free(peer);
I added the ASSERT more to ensure that this finalizer callback is not registered
for the case when we point
straight to the snapshot read only buffer.

In that case I setup the peer to be NULL and callback to NULL.

On 2012/09/05 20:33:50, cshapiro wrote:
> It is always safe to free NULL.  That is a no-op.

https://chromiumcodereview.appspot.com/10914050/diff/1/vm/snapshot.cc
File vm/snapshot.cc (right):

https://chromiumcodereview.appspot.com/10914050/diff/1/vm/snapshot.cc#newcode419
vm/snapshot.cc:419: reinterpret_cast<void*>(array),
On 2012/09/06 00:11:49, cshapiro wrote:
> make line 419 a NULL instead of non-NULL

Done.

Powered by Google App Engine
This is Rietveld 408576698