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

Issue 10891036: Reworked previous CL (ExternalStringGetPeer speed up) to avoid the (Closed)

Created:
8 years, 3 months ago by Tom Ball
Modified:
8 years, 3 months ago
Reviewers:
iposva, siva, Ivan Posva
CC:
reviews_dartlang.org
Visibility:
Public.

Description

Reworked previous CL (ExternalStringGetPeer speed up) to avoid the need for an 'extern "C"' declaration in raw_object.h, which broke the Windows builds. Instead, a peer() method is added to the RawExternalString*Object classes, and RawObject::ToAddr() is used to fetch the address of these raw instances. This avoids the need for Dart_ExternalStringGetPeer needing friend access to those classes. Committed: https://code.google.com/p/dart/source/detail?r=11540

Patch Set 1 #

Total comments: 2

Patch Set 2 : #

Patch Set 3 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+38 lines, -36 lines) Patch
M runtime/vm/dart_api_impl.cc View 1 1 chunk +35 lines, -28 lines 0 comments Download
M runtime/vm/raw_object.h View 1 3 chunks +3 lines, -8 lines 0 comments Download

Messages

Total messages: 5 (0 generated)
Tom Ball
The windows builds are failing due to a redeclaration of Dart_ExternalStringGetPeer function in the 'extern ...
8 years, 3 months ago (2012-08-29 19:18:49 UTC) #1
Tom Ball
8 years, 3 months ago (2012-08-29 19:27:37 UTC) #2
siva
http://codereview.chromium.org/10891036/diff/1/runtime/vm/raw_object.h File runtime/vm/raw_object.h (right): http://codereview.chromium.org/10891036/diff/1/runtime/vm/raw_object.h#newcode1134 runtime/vm/raw_object.h:1134: } We generally try to restrict this file to ...
8 years, 3 months ago (2012-08-29 19:39:10 UTC) #3
Ivan Posva
LGTM -ip
8 years, 3 months ago (2012-08-29 20:03:36 UTC) #4
Tom Ball
8 years, 3 months ago (2012-08-29 20:03:50 UTC) #5
http://codereview.chromium.org/10891036/diff/1/runtime/vm/raw_object.h
File runtime/vm/raw_object.h (right):

http://codereview.chromium.org/10891036/diff/1/runtime/vm/raw_object.h#newcod...
runtime/vm/raw_object.h:1134: }
Ivan helped me rework the original code to avoid needing these, so they're gone
now.

On 2012/08/29 19:39:11, asiva wrote:
> We generally try to restrict this file to just have data and avoid any
functions
> here.
> RawObject is a special case as it is used by GC and Snapshot code.

Powered by Google App Engine
This is Rietveld 408576698