http://codereview.chromium.org/39124/diff/1015/30
File src/debug.h (right):

http://codereview.chromium.org/39124/diff/1015/30#newcode574
Line 574: // If a preemption is pending clear it and store this
information.
On 2009/03/06 09:25:35, iposva wrote:
> Comment does not match with the code.
> Why do you need to test if prev_ is NULL?
> You are not recording the pending preemption in this case. Did you
miss to
> uncomment your setting of the preemption bit after testing? Otherwise
if you do
> not intend to set it, then you should remove the commented code.



Sorry, I forgot to un-comment code after doing some testing. Without
storing the preemption state when the debugger is first entered and
restoring it when it is left I can get starvation in some debugging
scenarios. Extended the comment with this information.

http://codereview.chromium.org/39124/diff/1015/30#newcode607
Line 607: if (prev_ == NULL && Debug::preemption_pending()) {
On 2009/03/06 09:25:35, iposva wrote:
> Please add a comment why you test for prev_ being NULL. For example:

> // Request preemption when leaving the last debugger entry and a
preemption had
> been recorded while debugging.

Clarified commet.

http://codereview.chromium.org/39124/diff/1015/32
File src/execution.cc (right):

http://codereview.chromium.org/39124/diff/1015/32#newcode527
Line 527: if (!Debug::InDebugger()) {
On 2009/03/06 09:25:35, iposva wrote:
> I would probably turn this around to not get the NOT in the test for
readability
> and then the comment also is closer to the described action.

Done.

http://codereview.chromium.org/39124/diff/1015/33
File test/cctest/test-debug.cc (right):

http://codereview.chromium.org/39124/diff/1015/33#newcode53
Line 53: using ::v8::internal::DeleteArray;
On 2009/03/06 09:25:35, iposva wrote:
> Why did you have to add these?

Removed - left over from temporary test cases.

http://codereview.chromium.org/39124

--~--~---------~--~----~------------~-------~--~----~
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
-~----------~----~----~----~------~----~------~--~---

Reply via email to