Define DEBUG for v8_optimized_debug=2

Define DEBUG for v8_optimized_debug=2
Thereby ensuring there is only a minimal performance regression vs. NDEBUG (now it's only about 10% slower rather than ~2x).

R=jkummerow@chromium.org, mstarzinger@chromium.org

Committed: http://code.google.com/p/v8/source/detail?r=17392

[danno, 2013-10-24 10:49:24]

PTAL

[Jakob Kummerow, 2013-10-24 13:52:09]

Drive-by nits.

High-level: I liked having a mode where we have compiler optimizations but slow
asserts. So I think OPTIMIZED_DEBUG should be #defined in v8_optimized_debug=2
mode instead of NDEBUG, rather than in the =1 mode. (Remember to update the CL
description accordingly if you change this.)

Other than that LGTM :-)

https://codereview.chromium.org/39183004/diff/110001/src/checks.h
File src/checks.h (right):

https://codereview.chromium.org/39183004/diff/110001/src/checks.h#newcode283
src/checks.h:283: #if ENABLE_SLOW_ASSERTS
s/if/ifdef/?

https://codereview.chromium.org/39183004/diff/110001/src/incremental-marking.cc
File src/incremental-marking.cc (right):

https://codereview.chromium.org/39183004/diff/110001/src/incremental-marking....
src/incremental-marking.cc:731: #if ENABLE_SLOW_ASSERTS
s/if/ifdef/

[danno, 2013-10-25 07:56:53]

please take another look

[Michael Starzinger, 2013-10-25 08:21:16]

LGTM. I like. Just one more nit.

https://codereview.chromium.org/39183004/diff/180001/src/checks.h
File src/checks.h (right):

https://codereview.chromium.org/39183004/diff/180001/src/checks.h#newcode306
src/checks.h:306: #define SLOW_ASSERT(condition) ((void) 0)
nit: Let's move the definition of SLOW assert out of this block up to line 284
and 286 respectively. Makes it easier to read and the conditions less cascaded.

[danno, 2013-10-25 11:10:41]

Committed patchset #9 manually as r17392 (presubmit successful).

[Dirk Pranke, 2013-10-30 00:43:20]

On 2013/10/25 11:10:41, danno wrote:
> Committed patchset #9 manually as r17392 (presubmit successful).

Assuming this did actually have the desired effect, it looks like the cycle time
increase was not large; somewhere in the 0-20% range (hard to tell b/c of the
noise and given the lame way the buildbot /stats numbers are reported). Such a
delay is quite acceptable.

Do you have a favorite test to run to validate that the right asserts are indeed
firing, or should I just hack some buggy code in?

[Jakob Kummerow, 2013-10-30 09:31:40]

On 2013/10/30 00:43:20, Dirk Pranke wrote:
> Do you have a favorite test to run to validate that the right asserts are
indeed
> firing, or should I just hack some buggy code in?

Thanks for offering help, but we're reasonably sure that the ASSERTs do fire as
intended. See e.g.:
https://codereview.chromium.org/46883008
https://codereview.chromium.org/47513015
https://codereview.chromium.org/49433002
https://codereview.chromium.org/49103002
https://codereview.chromium.org/48303004
https://codereview.chromium.org/48463002
and of course https://codereview.chromium.org/47543006/#msg6, which might also
explain why you're not seeing much of a performance impact.