@svenpanne
We already allow execution of "arbitrary C++ code" on VM thread with
SetDebugEventListener API.
https://codereview.chromium.org/812583003/diff/40001/include/v8-debug.h
File include/v8-debug.h (right):
https://codereview.chromium.org/812583003/diff/40001/include/v8-debug.h#newcode180
include/v8-debug.h:180: class Task {
On 2014/12/18 08:27:50, Sven Panne wrote:
We already have v8::Task, why do we need another one?
In general, it's a pity that we can't use std::function yet.
Sorry, missed the v8::Task.
https://codereview.chromium.org/812583003/diff/40001/include/v8-debug.h#newcode188
include/v8-debug.h:188: // VM thread. The VM takes ownership of the
task.
On 2014/12/18 08:27:51, Sven Panne wrote:
Please extend the comment a bit, describing that the task is run at
most once
and deleted after that. Another thing which needs a description: If it
is not
run, when exactly does it get destroyed?
Done.
https://codereview.chromium.org/812583003/diff/40001/src/debug.h
File src/debug.h (right):
https://codereview.chromium.org/812583003/diff/40001/src/debug.h#newcode328
src/debug.h:328: bool IsEmpty() const;
On 2014/12/18 08:27:51, Sven Panne wrote:
Not related to this CL, but anyway: Why is it safe to split Dequeue
into
IsEmpty/Get here? Looking at the call sites, it's far from clear that
there
can't be a race condition.
While it's not related to this CL ;-), I think it is safe provided e.g.
IsEmpty/Get is used on the consumer side only. In other words once
IsEmpty returns false it keeps that state until Get is called.
https://codereview.chromium.org/812583003/diff/40001/src/debug.h#newcode340
src/debug.h:340: class LockingTaskQueue BASE_EMBEDDED {
On 2014/12/18 08:27:51, Sven Panne wrote:
Remove BASE_EMBEDDED, it's nonsense, anyway...
Done.
https://codereview.chromium.org/812583003/diff/40001/src/debug.h#newcode350
src/debug.h:350: mutable base::Mutex mutex_;
On 2014/12/18 08:27:51, Sven Panne wrote:
Without having a single const function, why do we need mutable?
We don't. Removed.
https://codereview.chromium.org/812583003/diff/40001/src/debug.h#newcode351
src/debug.h:351: DISALLOW_COPY_AND_ASSIGN(LockingTaskQueue);
On 2014/12/18 08:27:51, Sven Panne wrote:
Strictly speaking this is not necessary and just noise, Mutex itself
can't be
copied/assigned already.
I don't want it to be depended on the set of fields. E.g. a field may
change at some point.
https://codereview.chromium.org/812583003/diff/40001/test/cctest/test-debug.cc
File test/cctest/test-debug.cc (right):
https://codereview.chromium.org/812583003/diff/40001/test/cctest/test-debug.cc#newcode7436
test/cctest/test-debug.cc:7436: // Delaying the task increases
probability the task is injected while the
On 2014/12/18 04:11:57, Benedikt Meurer wrote:
This is rather fragile. Can we have a more robust test case?
This test case it robust. It does not fail/flake depending on the
timeout value.
By adding the timeout I just wanted to increase the probability the task
is fired when VM is already running a pure-JS tight loop.
However, as Yury pointed out in an offline chat, the purpose of this
test is not to check the interrupts work for tight loops. But to check
that the task gets executed.
So I remove the Sleep once you insist.
https://codereview.chromium.org/812583003/diff/40001/test/cctest/test-debug.cc#newcode7436
test/cctest/test-debug.cc:7436: // Delaying the task increases
probability the task is injected while the
On 2014/12/18 08:27:51, Sven Panne wrote:
On 2014/12/18 04:11:57, Benedikt Meurer wrote:
> This is rather fragile. Can we have a more robust test case?
I would even go a step further: Without proper synchronization, this
is not
LGTM. We don't want to have "CPUProfiler reloaded" with all those
flakes on the
bots... ;-)
Again, this Sleep does not lead to flakiness. There's no race. Instead
it made a better test coverage of time moments the task is injected.
Anyway I removed it, once it's so frightening.
https://codereview.chromium.org/812583003/
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
