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

Issue 1277473004: Safe and efficient stack-limit based interrupt checking in C++. (Closed)

Created:
5 years, 4 months ago by koda
Modified:
5 years, 4 months ago
Reviewers:
Ivan Posva
CC:
reviews_dartlang.org, vm-dev_dartlang.org
Base URL:
git@github.com:dart-lang/sdk.git@master
Target Ref:
refs/heads/master
Visibility:
Public.

Description

Safe and efficient stack-limit based interrupt checking in C++. Remove potentially dangerous (but currently unused) Isolate::stack_limit() accessor. Add method to test whether interrupts are scheduled, using relaxed memory ordering. Document and test correct usage by adding a new unit test. This will be used to periodically check for safepoints in C++ (which will then be equivalent to our generated code). BUG= R=iposva@google.com Committed: https://github.com/dart-lang/sdk/commit/086329fd9c6901ae70ca4819ff9641e34329741b

Patch Set 1 #

Patch Set 2 : More general interfaces; comment. #

Patch Set 3 : Comments. #

Total comments: 2

Patch Set 4 : Mac and android. #

Patch Set 5 : Add unit test. #

Patch Set 6 : Actually add the unit test. #

Patch Set 7 : More comments in test. #

Patch Set 8 : Explicitly signal task ready. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+258 lines, -4 lines) Patch
M runtime/vm/atomic.h View 1 1 chunk +4 lines, -0 lines 0 comments Download
M runtime/vm/atomic_android.h View 1 2 3 2 chunks +16 lines, -0 lines 0 comments Download
M runtime/vm/atomic_linux.h View 1 2 2 chunks +16 lines, -0 lines 0 comments Download
M runtime/vm/atomic_macos.h View 1 2 3 2 chunks +16 lines, -0 lines 0 comments Download
A runtime/vm/atomic_test.cc View 1 2 3 4 5 1 chunk +36 lines, -0 lines 0 comments Download
M runtime/vm/atomic_win.h View 1 2 3 2 chunks +20 lines, -0 lines 0 comments Download
M runtime/vm/isolate.h View 1 2 chunks +17 lines, -4 lines 0 comments Download
M runtime/vm/isolate_test.cc View 1 2 3 4 5 6 7 2 chunks +132 lines, -0 lines 0 comments Download
M runtime/vm/vm_sources.gypi View 1 2 3 4 1 chunk +1 line, -0 lines 0 comments Download

Messages

Total messages: 9 (2 generated)
koda
5 years, 4 months ago (2015-08-07 20:41:31 UTC) #2
Ivan Posva
What about Mac? -Ivan https://codereview.chromium.org/1277473004/diff/40001/runtime/vm/atomic_win.h File runtime/vm/atomic_win.h (right): https://codereview.chromium.org/1277473004/diff/40001/runtime/vm/atomic_win.h#newcode34 runtime/vm/atomic_win.h:34: InterlockedAdd64(reinterpret_cast<LONGLONG*>(p), delta)) - 1; What ...
5 years, 4 months ago (2015-08-07 20:55:46 UTC) #3
koda
Updated mac and android. PTAL https://codereview.chromium.org/1277473004/diff/40001/runtime/vm/atomic_win.h File runtime/vm/atomic_win.h (right): https://codereview.chromium.org/1277473004/diff/40001/runtime/vm/atomic_win.h#newcode34 runtime/vm/atomic_win.h:34: InterlockedAdd64(reinterpret_cast<LONGLONG*>(p), delta)) - 1; ...
5 years, 4 months ago (2015-08-07 21:05:22 UTC) #4
koda
Added unit test.
5 years, 4 months ago (2015-08-07 21:18:42 UTC) #5
koda
Updated with explicit check for tasks to get ready, to rule out any unintentional safepoint ...
5 years, 4 months ago (2015-08-10 20:26:16 UTC) #7
Ivan Posva
LGTM -ip
5 years, 4 months ago (2015-08-11 18:32:05 UTC) #8
koda
5 years, 4 months ago (2015-08-11 19:38:01 UTC) #9
Message was sent while issue was closed.
Committed patchset #8 (id:160001) manually as
086329fd9c6901ae70ca4819ff9641e34329741b (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698