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

Issue 11421117: Fix for issue 6359 - Make snapshots platform independent. (Closed)

Created:
8 years ago by siva
Modified:
8 years ago
Reviewers:
srdjan
CC:
reviews_dartlang.org, vm-dev_dartlang.org
Visibility:
Public.

Description

Fix for issue 6359 - Make snapshots platform independent. The issue previously was that an instance size was stored a size in bytes, similarly field offsets were stored as an offset in bytes. This caused issues when generating the snapshot on one platform and reading it on another platform. The VM has been changed to store the instance size as a size in words and the field offset as an offset in words. Committed: https://code.google.com/p/dart/source/detail?r=15538

Patch Set 1 #

Patch Set 2 : #

Patch Set 3 : #

Total comments: 3

Patch Set 4 : #

Patch Set 5 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+101 lines, -51 lines) Patch
M vm/heap_profiler.cc View 1 2 3 4 1 chunk +1 line, -1 line 0 comments Download
M vm/object.h View 1 2 3 4 4 chunks +42 lines, -22 lines 0 comments Download
M vm/object.cc View 1 2 3 4 3 chunks +4 lines, -3 lines 0 comments Download
M vm/raw_object.h View 1 2 3 4 2 chunks +4 lines, -4 lines 0 comments Download
M vm/raw_object.cc View 1 2 3 4 2 chunks +3 lines, -2 lines 0 comments Download
M vm/raw_object_snapshot.cc View 1 2 3 4 2 chunks +6 lines, -6 lines 0 comments Download
M vm/snapshot.h View 1 2 3 4 1 chunk +1 line, -0 lines 0 comments Download
M vm/snapshot.cc View 1 2 3 4 9 chunks +36 lines, -9 lines 0 comments Download
M vm/stub_code_ia32.cc View 1 2 3 4 1 chunk +2 lines, -2 lines 0 comments Download
M vm/stub_code_x64.cc View 1 2 3 4 1 chunk +2 lines, -2 lines 0 comments Download

Messages

Total messages: 3 (0 generated)
siva
8 years ago (2012-11-28 03:02:37 UTC) #1
srdjan
LGTM after clarifying the left/right shift on signed values. https://codereview.chromium.org/11421117/diff/2003/vm/snapshot.cc File vm/snapshot.cc (right): https://codereview.chromium.org/11421117/diff/2003/vm/snapshot.cc#newcode1038 vm/snapshot.cc:1038: ...
8 years ago (2012-11-29 00:49:22 UTC) #2
siva
8 years ago (2012-11-29 03:08:20 UTC) #3
Changed shift operations to mul/div.

https://codereview.chromium.org/11421117/diff/2003/vm/snapshot.cc
File vm/snapshot.cc (right):

https://codereview.chromium.org/11421117/diff/2003/vm/snapshot.cc#newcode1038
vm/snapshot.cc:1038: const intptr_t kBits = 62;
Strictly speaking this not kMintBits as you represent
full 64 bit values as Mint values. These are more
SmiBits for 64 bit architectures.

I could rename them to kSmi64Bits and leave them here.

On 2012/11/29 00:49:22, srdjan wrote:
> Should this be kMintBits, defined at similar place as kSmiBits?

Powered by Google App Engine
This is Rietveld 408576698