Revision: 6415
Author: [email protected]
Date: Thu Jan 20 04:32:43 2011
Log: Make 'with' mark only variables occurring in the body as used.
Before, we conservatively marked every variable in a scope as used if the
scope contained 'with'. Instead, just mark the variables occurring in the
body of the with. This avoids marking 'arguments' as used whenever 'with'
occurs, which incurs an extra performance penalty (a use of arguments is
seen as an instruction to redirect all parameter accesses to the arguments
object).
Review URL: http://codereview.chromium.org/6357007
http://code.google.com/p/v8/source/detail?r=6415
Modified:
/branches/bleeding_edge/src/scopes.cc
/branches/bleeding_edge/test/mjsunit/debug-evaluate-locals.js
=======================================
--- /branches/bleeding_edge/src/scopes.cc Wed Jan 19 00:16:17 2011
+++ /branches/bleeding_edge/src/scopes.cc Thu Jan 20 04:32:43 2011
@@ -726,6 +726,7 @@
// Note that we must do a lookup anyway, because if we find one,
// we must mark that variable as potentially accessed from this
// inner scope (the property may not be in the 'with' object).
+ if (var != NULL) var->set_is_used(true);
var = NonLocal(proxy->name(), Variable::DYNAMIC);
} else {
@@ -833,8 +834,8 @@
// visible name.
if ((var->is_this() || var->name()->length() > 0) &&
(var->is_accessed_from_inner_scope() ||
- scope_calls_eval_ || inner_scope_calls_eval_ ||
- scope_contains_with_)) {
+ scope_calls_eval_ ||
+ inner_scope_calls_eval_)) {
var->set_is_used(true);
}
// Global variables do not need to be allocated.
=======================================
--- /branches/bleeding_edge/test/mjsunit/debug-evaluate-locals.js Tue Dec
7 03:01:02 2010
+++ /branches/bleeding_edge/test/mjsunit/debug-evaluate-locals.js Thu Jan
20 04:32:43 2011
@@ -34,18 +34,18 @@
function checkFrame0(name, value) {
- assertTrue(name == 'a' || name == 'b');
+ assertTrue(name == 'a' || name == 'b', 'check name');
if (name == 'a') {
assertEquals(1, value);
- }
- if (name == 'b') {
+ } else if (name == 'b') {
assertEquals(2, value);
}
}
function checkFrame1(name, value) {
- assertTrue(name == '.arguments' || name == 'a');
+ assertTrue(name == '.arguments' || name == 'arguments' || name == 'a',
+ 'check name');
if (name == 'a') {
assertEquals(3, value);
}
@@ -53,8 +53,7 @@
function checkFrame2(name, value) {
- assertTrue(name == '.arguments' || name == 'a' ||
- name == 'arguments' || name == 'b');
+ assertTrue(name == 'a' || name == 'b');
if (name == 'a') {
assertEquals(5, value);
}
@@ -73,18 +72,17 @@
checkFrame0(frame0.localName(0), frame0.localValue(0).value());
checkFrame0(frame0.localName(1), frame0.localValue(1).value());
- // Frame 1 has normal variable a (and the .arguments variable).
+ // Frame 1 has normal variables a and arguments (and the .arguments
+ // variable).
var frame1 = exec_state.frame(1);
checkFrame1(frame1.localName(0), frame1.localValue(0).value());
checkFrame1(frame1.localName(1), frame1.localValue(1).value());
-
- // Frame 2 has normal variables a and b (and both the .arguments and
- // arguments variable).
+ checkFrame1(frame1.localName(2), frame1.localValue(2).value());
+
+ // Frame 2 has normal variables a and b.
var frame2 = exec_state.frame(2);
checkFrame2(frame2.localName(0), frame2.localValue(0).value());
checkFrame2(frame2.localName(1), frame2.localValue(1).value());
- checkFrame2(frame2.localName(2), frame2.localValue(2).value());
- checkFrame2(frame2.localName(3), frame2.localValue(3).value());
// Evaluating a and b on frames 0, 1 and 2 produces 1, 2, 3, 4, 5
and 6.
assertEquals(1, exec_state.frame(0).evaluate('a').value());
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
