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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 1418833004: VM: Service isolate under precompilation. (Closed)

Created:
5 years, 1 month ago by rmacnak
Modified:
5 years, 1 month ago
Reviewers:
Cutch, Chinmay
CC:
Chinmay, Cutch, reviews_dartlang.org, ricow1, turnidge, vm-dev_dartlang.org
Base URL:
git@github.com:dart-lang/sdk.git@master
Target Ref:
refs/heads/master
Visibility:
Public.

Description

VM: Service isolate under precompilation. - Include the embedder's portion of the service isolate's Dart code in precompiled snapshots. Swap it in as the root library when running precompiled instead of loading from source. - Make linter happy with gen_snapshot. - Fix profiler to consider Instructions in the VM isolate may have their corresponding Code in the regular isolate. - Make non-functional service RPCs fail cleanly instead of crashing. BUG=http://dartbug.com/24651 R=johnmccutchan@google.com Committed: https://github.com/dart-lang/sdk/commit/37dd876bf1a6e6854abc85ebdd9c513e03743580

Patch Set 1 #

Patch Set 2 : #

Patch Set 3 : #

Total comments: 4

Patch Set 4 : #

Patch Set 5 : typo #

Total comments: 2

Patch Set 6 : sync, +doc, +unittest #

Patch Set 7 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+266 lines, -94 lines) Patch
M runtime/bin/gen_snapshot.cc View 1 2 3 2 chunks +46 lines, -49 lines 0 comments Download
M runtime/bin/main.cc View 1 2 3 4 4 chunks +17 lines, -1 line 0 comments Download
M runtime/bin/vmservice_dartium.cc View 1 2 3 1 chunk +2 lines, -1 line 0 comments Download
M runtime/bin/vmservice_impl.h View 2 chunks +6 lines, -1 line 0 comments Download
M runtime/bin/vmservice_impl.cc View 1 2 3 4 5 6 5 chunks +49 lines, -15 lines 0 comments Download
M runtime/include/dart_api.h View 1 2 3 4 5 1 chunk +9 lines, -0 lines 0 comments Download
M runtime/vm/dart_api_impl.cc View 1 1 chunk +12 lines, -0 lines 0 comments Download
M runtime/vm/dart_api_impl_test.cc View 1 2 3 4 5 1 chunk +20 lines, -0 lines 0 comments Download
M runtime/vm/object.h View 1 2 3 4 5 1 chunk +15 lines, -0 lines 0 comments Download
M runtime/vm/object.cc View 1 2 3 4 5 1 chunk +22 lines, -11 lines 0 comments Download
M runtime/vm/precompiler.cc View 1 2 3 4 5 1 chunk +3 lines, -2 lines 0 comments Download
M runtime/vm/profiler_service.cc View 1 chunk +10 lines, -0 lines 0 comments Download
M runtime/vm/service.cc View 9 chunks +45 lines, -0 lines 0 comments Download
M runtime/vm/service_event.cc View 1 chunk +1 line, -1 line 0 comments Download
M runtime/vm/service_isolate.cc View 1 2 3 4 5 1 chunk +0 lines, -4 lines 0 comments Download
M tools/precompilation/test_linux.sh View 1 1 chunk +3 lines, -3 lines 0 comments Download
M tools/precompilation/test_linux_simarm.sh View 1 1 chunk +3 lines, -3 lines 0 comments Download
M tools/precompilation/test_macos.sh View 1 1 chunk +3 lines, -3 lines 0 comments Download

Messages

Total messages: 11 (5 generated)
rmacnak
5 years, 1 month ago (2015-10-28 22:49:24 UTC) #5
Chinmay
Patching this in locally enables observatory on precompiled variants! https://codereview.chromium.org/1418833004/diff/40001/runtime/bin/main.cc File runtime/bin/main.cc (right): https://codereview.chromium.org/1418833004/diff/40001/runtime/bin/main.cc#newcode1235 runtime/bin/main.cc:1235: ...
5 years, 1 month ago (2015-10-28 23:20:48 UTC) #7
rmacnak
https://codereview.chromium.org/1418833004/diff/40001/runtime/bin/main.cc File runtime/bin/main.cc (right): https://codereview.chromium.org/1418833004/diff/40001/runtime/bin/main.cc#newcode1235 runtime/bin/main.cc:1235: "VM servie loading failed: %s\n", On 2015/10/28 23:20:48, Chinmay ...
5 years, 1 month ago (2015-10-28 23:55:41 UTC) #8
Cutch
LGTM after adding doc comments and test for new API entry. https://codereview.chromium.org/1418833004/diff/80001/runtime/include/dart_api.h File runtime/include/dart_api.h (right): ...
5 years, 1 month ago (2015-10-29 21:15:32 UTC) #9
rmacnak
https://codereview.chromium.org/1418833004/diff/80001/runtime/include/dart_api.h File runtime/include/dart_api.h (right): https://codereview.chromium.org/1418833004/diff/80001/runtime/include/dart_api.h#newcode2717 runtime/include/dart_api.h:2717: DART_EXPORT Dart_Handle Dart_SetRootLibrary(Dart_Handle library); On 2015/10/29 21:15:32, Cutch wrote: ...
5 years, 1 month ago (2015-10-29 22:07:32 UTC) #10
rmacnak
5 years, 1 month ago (2015-10-29 22:15:50 UTC) #11
Message was sent while issue was closed. 
Committed patchset #7 (id:120001) manually as
37dd876bf1a6e6854abc85ebdd9c513e03743580 (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698