Disable EXTRA_CHECKS in Release (issue 11275324)

[aba...@chromium.org]

Reviewers: Sven Panne,

Message:
This is a simple patch that makes dom-traverse around 4% faster on my Mac.
I'm
not very good at GYP, so please let me know if there's a better way to
structure
this change.

Thanks!

Description:
Disable EXTRA_CHECKS in Release

This patch causes V8 to disable EXTRA_CHECKS when building in release. You
can
still enable the checks in release using a GYP flag.

This patch speeds up Dromeo's dom-traverse by around 4%.


Please review this at https://codereview.chromium.org/11275324/

SVN Base: git://github.com/v8/v8.git@bleeding_edge

Affected files:
M build/common.gypi


Index: build/common.gypi
diff --git a/build/common.gypi b/build/common.gypi
index
9559d98db252b7df8d7fdf75fc268f335ae27c72..46972c6763b1acf7b1bb41e69231f874191dd4f7
100644
--- a/build/common.gypi
+++ b/build/common.gypi
@@ -70,9 +70,6 @@

'v8_enable_disassembler%': 0,

- # Enable extra checks in API functions and other strategic places.
- 'v8_enable_extra_checks%': 1,
-
'v8_enable_gdbjit%': 0,

'v8_object_print%': 0,
@@ -114,9 +111,6 @@
['v8_enable_disassembler==1', {
'defines': ['ENABLE_DISASSEMBLER',],
}],
- ['v8_enable_extra_checks==1', {
- 'defines': ['ENABLE_EXTRA_CHECKS',],
- }],
['v8_enable_gdbjit==1', {
'defines': ['ENABLE_GDB_JIT_INTERFACE',],
}],
@@ -336,6 +330,9 @@
], # conditions
'configurations': {
'Debug': {
+ 'variables': {
+ 'v8_enable_extra_checks%': 1,
+ },
'defines': [
'DEBUG',
'ENABLE_DISASSEMBLER',
@@ -360,6 +357,9 @@
},
},
'conditions': [
+ ['v8_enable_extra_checks==1', {
+ 'defines': ['ENABLE_EXTRA_CHECKS',],
+ }],
['OS=="linux" or OS=="freebsd" or OS=="openbsd" or
OS=="netbsd"', {
'cflags':
[ '-Wall', '<(werror)', '-W', '-Wno-unused-parameter',
'-Wnon-virtual-dtor', '-Woverloaded-virtual' ],
@@ -381,7 +381,13 @@
],
}, # Debug
'Release': {
+ 'variables': {
+ 'v8_enable_extra_checks%': 0,
+ },
'conditions': [
+ ['v8_enable_extra_checks==1', {
+ 'defines': ['ENABLE_EXTRA_CHECKS',],
+ }],
['OS=="linux" or OS=="freebsd" or OS=="openbsd" or OS=="netbsd" \
or OS=="android"', {
'cflags!': [

[sven...@chromium.org]

[Looping in our GYP master, too]

EXTRA_CHECKS was introduced mainly to catch bugs in native
functions/callbacks
when they return non-JavaScript objects. This happened quite often some
time ago
and was *very* hard to debug, because the symptom (crash in V8) was very far
from the reason (e.g. buggy string cache in the Chromium bindings). It's
intention was to be relatively low-overhead, so we can ship Chrome with this
flag enabled.

A sensible approach might be shipping canaries and dev-builds with
EXTRA_CHECKS
enabled, but the final release without it. I am not sure how well this
integrates into our release mechanisms. @danno: What do you think?

https://codereview.chromium.org/11275324/

[aba...@chromium.org]

The release managers don't like shipping different builds to different
channels
because it makes the release process less predictable. I've long thought we
should be able to ship canary with some sorts of extra error checking.

https://codereview.chromium.org/11275324/

[da...@chromium.org]

We landed the extra checks with the assumption that they wouldn't negatively
impact perf (at least not significantly). Since that's true, and AFAIK
there's
been no remaining API abuse identified by crash dumps, we should disable it.


https://codereview.chromium.org/11275324/

[aba...@chromium.org]

If the API abuse comes back, we can turn them back on to try and diagnose.

https://codereview.chromium.org/11275324/

[jkum...@chromium.org]

LGTM.

I would've liked to keep these checks in for ToT/Canary builds, but I can
understand that the Release team would prefer to avoid that.

I'll land this.

https://codereview.chromium.org/11275324/