Browse Source

Merge pull request #1146 from svaarala/improve-varmap-dropping

Allow dropping of _Varmap when only safe slow path variable reads are present
pull/1155/head
Sami Vaarala 8 years ago
committed by GitHub
parent
commit
1dd448809e
  1. 4
      RELEASES.rst
  2. 4
      doc/testcase-known-issues.yaml
  3. 81
      src-input/duk_js_compiler.c
  4. 43
      src-input/duk_js_compiler.h
  5. 2
      tests/ecmascript/test-arguments-magic-sync.js
  6. 6
      tests/ecmascript/test-dev-func-formals-drop.js
  7. 222
      tests/ecmascript/test-dev-func-varmap-drop.js
  8. 10
      tests/ecmascript/util-object.js
  9. 57
      tests/knownissues/test-dev-func-varmap-drop-1.txt

4
RELEASES.rst

@ -2146,7 +2146,9 @@ Planned
internal value stack access improvements (GH-1058); shared bitpacked string
format for heap and thread initialization data (GH-1119); explicit
lexenv/varenv fields in duk_hcompfunc struct (GH-1132); omit duk_hcompfunc
_Formals array when it is safe to do so (GH-1141)
_Formals array when it is safe to do so (GH-1141); omit duk_hcompfunc
_Varmap in more cases when it is safe to do so (GH-1146); reduce initial
bytecode allocation in Ecmascript compiler for low memory targets (GH-1146)
* Internal change: rework shared internal string handling so that shared
strings are plain string constants used in macro values, rather than

4
doc/testcase-known-issues.yaml

@ -163,6 +163,10 @@
test: "test-bi-typedarray-nan-handling.js"
specialoptions: "NaN sign is platform dependent"
# No longer relevant (at least with current test runs)
-
test: "test-dev-func-varmap-drop.js"
specialoptions: "debugger support must be disabled"
# Moved
# API testcase (tests/api/) known issues

81
src-input/duk_js_compiler.c

@ -60,7 +60,11 @@
#define DUK__MAX_TEMPS 0xffffL
/* Initial bytecode size allocation. */
#if defined(DUK_USE_PREFER_SIZE)
#define DUK__BC_INITIAL_INSTS 16
#else
#define DUK__BC_INITIAL_INSTS 256
#endif
#define DUK__RECURSION_INCREASE(comp_ctx,thr) do { \
DUK_DDD(DUK_DDDPRINT("RECURSION INCREASE: %s:%ld", (const char *) DUK_FILE_MACRO, (long) DUK_LINE_MACRO)); \
@ -660,6 +664,9 @@ DUK_LOCAL void duk__convert_to_func_template(duk_compiler_ctx *comp_ctx, duk_boo
duk_tval *tv;
duk_bool_t keep_varmap;
duk_bool_t keep_formals;
#if !defined(DUK_USE_DEBUGGER_SUPPORT)
duk_size_t formals_length;
#endif
DUK_DDD(DUK_DDDPRINT("converting duk_compiler_func to function/template"));
@ -823,25 +830,31 @@ DUK_LOCAL void duk__convert_to_func_template(duk_compiler_ctx *comp_ctx, duk_boo
/* [ ... res ] */
/* _Varmap: omitted if function is guaranteed not to do slow path identifier
* accesses or if it would turn out to be empty of actual register mappings
* after a cleanup. When debugging is enabled, we always need the varmap to
* be able to lookup variables at any point.
/* _Varmap: omitted if function is guaranteed not to do a slow path
* identifier access that might be caught by locally declared variables.
* The varmap can also be omitted if it turns out empty of actual
* register mappings after a cleanup. When debugging is enabled, we
* always need the varmap to be able to lookup variables at any point.
*/
#if defined(DUK_USE_DEBUGGER_SUPPORT)
DUK_DD(DUK_DDPRINT("keeping _Varmap because debugger support is enabled"));
keep_varmap = 1;
#else
keep_varmap =
func->id_access_slow || /* directly uses slow accesses */
if (func->id_access_slow_own || /* directly uses slow accesses that may match own variables */
func->id_access_arguments || /* accesses 'arguments' directly */
func->may_direct_eval || /* may indirectly slow access through a direct eval */
funcs_count > 0; /* has inner functions which may slow access (XXX: this can be optimized by looking at the inner functions) */
funcs_count > 0) { /* has inner functions which may slow access (XXX: this can be optimized by looking at the inner functions) */
DUK_DD(DUK_DDPRINT("keeping _Varmap because of direct eval, slow path access that may match local variables, or presence of inner functions"));
keep_varmap = 1;
} else {
DUK_DD(DUK_DDPRINT("dropping _Varmap"));
keep_varmap = 0;
}
#endif
if (keep_varmap) {
duk_int_t num_used;
DUK_DD(DUK_DDPRINT("keeping _Varmap"));
duk_dup(ctx, func->varmap_idx);
num_used = duk__cleanup_varmap(comp_ctx);
DUK_DDD(DUK_DDDPRINT("cleaned up varmap: %!T (num_used=%ld)",
@ -867,18 +880,29 @@ DUK_LOCAL void duk__convert_to_func_template(duk_compiler_ctx *comp_ctx, duk_boo
DUK_DD(DUK_DDPRINT("keeping _Formals because debugger support is enabled"));
keep_formals = 1;
#else
keep_formals =
func->id_access_arguments || /* accesses 'arguments', main reason to keep, could check strictness too */
func->may_direct_eval || /* direct eval -> may access 'arguments' via eval */
duk_get_length(ctx, func->argnames_idx) != (duk_size_t) h_res->nargs; /* nargs not enough for closure .length */
formals_length = duk_get_length(ctx, func->argnames_idx);
if (formals_length != (duk_size_t) h_res->nargs) {
/* Nargs not enough for closure .length: keep _Formals regardless
* of its length. Shouldn't happen in practice at the moment.
*/
DUK_DD(DUK_DDPRINT("keeping _Formals because _Formals.length != nargs"));
keep_formals = 1;
} else if ((func->id_access_arguments || func->may_direct_eval) &&
(formals_length > 0)) {
/* Direct eval (may access 'arguments') or accesses 'arguments'
* explicitly: keep _Formals unless it is zero length.
*/
DUK_DD(DUK_DDPRINT("keeping _Formals because of direct eval or explicit access to 'arguments', and _Formals.length != 0"));
keep_formals = 1;
} else {
DUK_DD(DUK_DDPRINT("omitting _Formals, nargs matches _Formals.length, so no properties added"));
keep_formals = 0;
}
#endif
if (keep_formals) {
DUK_DD(DUK_DDPRINT("keeping _Formals"));
duk_dup(ctx, func->argnames_idx);
duk_xdef_prop_stridx(ctx, -2, DUK_STRIDX_INT_FORMALS, DUK_PROPDESC_FLAGS_NONE);
} else {
DUK_DD(DUK_DDPRINT("omitting _Formals, nargs matches _Formals.length, so no properties added"));
}
/* name */
@ -2430,7 +2454,7 @@ DUK_LOCAL duk_reg_t duk__lookup_active_register_binding(duk_compiler_ctx *comp_c
if (comp_ctx->curr_func.with_depth > 0) {
DUK_DDD(DUK_DDDPRINT("identifier lookup inside a 'with' -> fall back to slow path"));
goto slow_path;
goto slow_path_own;
}
/*
@ -2447,16 +2471,33 @@ DUK_LOCAL duk_reg_t duk__lookup_active_register_binding(duk_compiler_ctx *comp_c
duk_pop(ctx);
} else {
duk_pop(ctx);
goto slow_path;
if (comp_ctx->curr_func.catch_depth > 0 || comp_ctx->curr_func.with_depth > 0) {
DUK_DDD(DUK_DDDPRINT("slow path access from inside a try-catch or with needs _Varmap"));
goto slow_path_own;
} else {
/* In this case we're doing a variable lookup that doesn't
* match our own variables, so _Varmap won't be needed at
* run time.
*/
DUK_DDD(DUK_DDDPRINT("slow path access outside of try-catch and with, no need for _Varmap"));
goto slow_path_notown;
}
}
DUK_DDD(DUK_DDDPRINT("identifier lookup -> reg %ld", (long) ret));
return ret;
slow_path:
DUK_DDD(DUK_DDDPRINT("identifier lookup -> slow path"));
slow_path_notown:
DUK_DDD(DUK_DDDPRINT("identifier lookup -> slow path, not own variable"));
comp_ctx->curr_func.id_access_slow = 1;
return (duk_reg_t) -1;
slow_path_own:
DUK_DDD(DUK_DDDPRINT("identifier lookup -> slow path, may be own variable"));
comp_ctx->curr_func.id_access_slow = 1;
comp_ctx->curr_func.id_access_slow_own = 1;
return (duk_reg_t) -1;
}
@ -7166,6 +7207,7 @@ DUK_LOCAL void duk__parse_func_body(duk_compiler_ctx *comp_ctx, duk_bool_t expec
func->may_direct_eval = 0;
func->id_access_arguments = 0;
func->id_access_slow = 0;
func->id_access_slow_own = 0;
func->reg_stmt_value = reg_stmt_value;
#if defined(DUK_USE_DEBUGGER_SUPPORT)
func->min_line = DUK_INT_MAX;
@ -7255,6 +7297,7 @@ DUK_LOCAL void duk__parse_func_body(duk_compiler_ctx *comp_ctx, duk_bool_t expec
/* XXX: init or assert catch depth etc -- all values */
func->id_access_arguments = 0;
func->id_access_slow = 0;
func->id_access_slow_own = 0;
/*
* Check function name validity now that we know strictness.

43
src-input/duk_js_compiler.h

@ -130,7 +130,7 @@ struct duk_compiler_func {
duk_hobject *h_argnames; /* array of formal argument names (-> _Formals) */
duk_hobject *h_varmap; /* variable map for pass 2 (identifier -> register number or null (unmapped)) */
/* value stack indices for tracking objects */
/* Value stack indices for tracking objects. */
/* code_idx: not needed */
duk_idx_t consts_idx;
duk_idx_t funcs_idx;
@ -140,24 +140,24 @@ struct duk_compiler_func {
duk_idx_t argnames_idx;
duk_idx_t varmap_idx;
/* temp reg handling */
/* Temp reg handling. */
duk_reg_t temp_first; /* first register that is a temporary (below: variables) */
duk_reg_t temp_next; /* next temporary register to allocate */
duk_reg_t temp_max; /* highest value of temp_reg (temp_max - 1 is highest used reg) */
/* shuffle registers if large number of regs/consts */
/* Shuffle registers if large number of regs/consts. */
duk_reg_t shuffle1;
duk_reg_t shuffle2;
duk_reg_t shuffle3;
/* stats for current expression being parsed */
/* Stats for current expression being parsed. */
duk_int_t nud_count;
duk_int_t led_count;
duk_int_t paren_level; /* parenthesis count, 0 = top level */
duk_bool_t expr_lhs; /* expression is left-hand-side compatible */
duk_bool_t allow_in; /* current paren level allows 'in' token */
/* misc */
/* Misc. */
duk_int_t stmt_next; /* statement id allocation (running counter) */
duk_int_t label_next; /* label id allocation (running counter) */
duk_int_t catch_depth; /* catch stack depth */
@ -170,22 +170,23 @@ struct duk_compiler_func {
duk_int_t max_line;
#endif
/* status booleans */
duk_bool_t is_function; /* is an actual function (not global/eval code) */
duk_bool_t is_eval; /* is eval code */
duk_bool_t is_global; /* is global code */
duk_bool_t is_setget; /* is a setter/getter */
duk_bool_t is_decl; /* is a function declaration (as opposed to function expression) */
duk_bool_t is_strict; /* function is strict */
duk_bool_t is_notail; /* function must not be tail called */
duk_bool_t in_directive_prologue; /* parsing in "directive prologue", recognize directives */
duk_bool_t in_scanning; /* parsing in "scanning" phase (first pass) */
duk_bool_t may_direct_eval; /* function may call direct eval */
duk_bool_t id_access_arguments; /* function refers to 'arguments' identifier */
duk_bool_t id_access_slow; /* function makes one or more slow path accesses */
duk_bool_t is_arguments_shadowed; /* argument/function declaration shadows 'arguments' */
duk_bool_t needs_shuffle; /* function needs shuffle registers */
duk_bool_t reject_regexp_in_adv; /* reject RegExp literal on next advance() call; needed for handling IdentifierName productions */
/* Status booleans. */
duk_uint8_t is_function; /* is an actual function (not global/eval code) */
duk_uint8_t is_eval; /* is eval code */
duk_uint8_t is_global; /* is global code */
duk_uint8_t is_setget; /* is a setter/getter */
duk_uint8_t is_decl; /* is a function declaration (as opposed to function expression) */
duk_uint8_t is_strict; /* function is strict */
duk_uint8_t is_notail; /* function must not be tail called */
duk_uint8_t in_directive_prologue; /* parsing in "directive prologue", recognize directives */
duk_uint8_t in_scanning; /* parsing in "scanning" phase (first pass) */
duk_uint8_t may_direct_eval; /* function may call direct eval */
duk_uint8_t id_access_arguments; /* function refers to 'arguments' identifier */
duk_uint8_t id_access_slow; /* function makes one or more slow path accesses that won't match own static variables */
duk_uint8_t id_access_slow_own; /* function makes one or more slow path accesses that may match own static variables */
duk_uint8_t is_arguments_shadowed; /* argument/function declaration shadows 'arguments' */
duk_uint8_t needs_shuffle; /* function needs shuffle registers */
duk_uint8_t reject_regexp_in_adv; /* reject RegExp literal on next advance() call; needed for handling IdentifierName productions */
};
struct duk_compiler_ctx {

2
tests/ecmascript/test-arguments-magic-sync.js

@ -57,5 +57,5 @@ function f(x) {
try {
f(1);
} catch (e) {
print(e);
print(e.stack || e);
}

6
tests/ecmascript/test-dev-hcompfunc-length-handling.js → tests/ecmascript/test-dev-func-formals-drop.js

@ -5,6 +5,12 @@
* is always kept.
*/
/*---
{
"custom": true
}
---*/
/*===
3
foo bar quux

222
tests/ecmascript/test-dev-func-varmap-drop.js

@ -0,0 +1,222 @@
/*
* Function _Varmap can be dropped when there can be no runtime need for it.
* Exercise current conditions. Needs to be executed with debugger support
* disabled (otherwise _Varmap will always be present).
*/
/*---
{
"custom": true
}
---*/
/*@include util-object.js@*/
/*===
- pr(arg)
hello
prop count diff: 0
- Math.PI
3.141592653589793
prop count diff: 0
3.141592653589793
prop count diff: 0
- direct eval
3.141592653589793
prop count diff: 2
3.141592653589793
prop count diff: 0
- inner function
3.141592653589793
prop count diff: 1
3.141592653589793
prop count diff: 1
- try-catch without a slow path access in catch clause
3.141592653589793
prop count diff: 1
3.141592653589793
prop count diff: 0
- try-catch with a slow path access in catch clause
3.141592653589793
prop count diff: 1
3.141592653589793
prop count diff: 0
- with statement without a slow path access
3.141592653589793
prop count diff: 0
3.141592653589793
prop count diff: 0
- with statement with a slow path access
3.141592653589793
prop count diff: 1
3.141592653589793
prop count diff: 0
- arguments object access
3.141592653589793
object
prop count diff: 2
3.141592653589793
object
prop count diff: 0
- test eval creating a local variable for a function without _Varmap
123
prop count diff: 2
123
prop count diff: 0
- inner function without varmap, access outer function variables
123
prop count diff: 1
123
prop count diff: 1
===*/
function test() {
var func;
var propsBaseline;
// Baseline case.
func = function foo() {};
func();
propsBase = getObjectPropertyCount(func);
function printPropDiff() {
print('prop count diff: ' + (getObjectPropertyCount(func) - propsBase));
}
// Basic case: all identifier accesses are to register variables
// and no slow path accesses are made.
print('- pr(arg)');
func = function foo(pr, arg) { pr(arg); };
func(print, 'hello');
printPropDiff();
// Quite basic case: print and Math are slow path accesses that cannot
// match statically declared locals.
print('- Math.PI');
func = function foo(a,b) { print(Math.PI); };
func();
printPropDiff();
func = function foo() { print(Math.PI); };
func();
printPropDiff();
// Eval in the function might introduce new variable declarations at
// run time, so presence of a (direct) eval requires _Varmap.
// If there are no register-bound variables in the _Varmap,
// it can still be omitted.
//
// Note that eval() here also causes _Formals to be kept, unless there
// are no formal arguments at all.
print('- direct eval');
func = function foo(a,b) { print(Math.PI); eval(1); };
func();
printPropDiff();
func = function foo() { print(Math.PI); eval(1); };
func();
printPropDiff();
// Inner functions could potentially need the _Varmap so any inner
// function declarations (even if not invoked) cause _Varmap to be
// kept at present. It would be possible to detect if the inner
// function(s) actually make any dangerous slow path accesses, and
// only keep the outer function's _Varmap if really necessary.
// However, the compiler doesn't have enough state right now to do
// this.
//
// Note that an inner function always creates a register based local
// variable, so that _Varmap is never empty even if there are no formal
// arguments or variable declarations/consts.
print('- inner function');
func = function foo(a,b) { print(Math.PI); function inner() {}; };
func();
printPropDiff();
func = function foo() { print(Math.PI); function inner() {}; };
func();
printPropDiff();
// Try-catch statement's catch variable is handled using a dynamic
// scope object. Identifier accesses within the catch block use slow
// path access, and for simplicity the _Varmap is kept if any slow
// path accesses are made there. _Varmap could be omitted if the catch
// clause doesn't perform a slow path access in practice -- but this
// won't happen now because the automatic catch clause prologue will
// perform a slow path access to load the catch variable into a register.
//
// When there are no register-based declarations (no formals, no variables,
// no inner functions) _Varmap is still omitted because it is empty.
print('- try-catch without a slow path access in catch clause');
func = function foo(a,b) { print(Math.PI); try { throw new Error('aiee'); } catch (e) {} };
func();
printPropDiff();
func = function foo() { print(Math.PI); try { throw new Error('aiee'); } catch (e) {} };
func();
printPropDiff();
// But with a slow path access in the catch clause the _Varmap is
// kept, even if there's no way for the catch clause to run.
print('- try-catch with a slow path access in catch clause');
func = function foo(a,b) { print(Math.PI); try {} catch (e) { print(Math); } };
func();
printPropDiff();
func = function foo() { print(Math.PI); try {} catch (e) { print(Math); } };
func();
printPropDiff();
// Presence of a with statement alone is not enough to cause _Varmap to be
// kept.
print('- with statement without a slow path access');
func = function foo(a,b) { print(Math.PI); with({}) {} };
func();
printPropDiff();
func = function foo() { print(Math.PI); with({}) {} };
func();
printPropDiff();
// But any variable access inside the with statement causes _Varmap to
// be kept, unless there are no register-bound variables at all.
print('- with statement with a slow path access');
func = function foo(a,b) { print(Math.PI); with({}) { Math; } };
func();
printPropDiff();
func = function foo() { print(Math.PI); with({}) { Math; } };
func();
printPropDiff();
// Access to 'arguments' specifically causes _Varmap to be kept. It's
// handled as a special case in the compiler, so also worth testing
// separately.
print('- arguments object access');
func = function foo(a,b) { print(Math.PI); print(typeof arguments); };
func();
printPropDiff();
func = function foo() { print(Math.PI); print(typeof arguments); };
func();
printPropDiff();
// Regardless of the optimizations above, eval must be able to create
// local variables (even if _Varmap is dropped). This happens in the
// latter case without formals (so that _Formals.length is 0 and there
// are no register bound variables).
print('- test eval creating a local variable for a function without _Varmap');
func = function foo(a,b) { eval('var z = 123;'); print(z); };
func();
printPropDiff();
func = function foo() { eval('var z = 123;'); print(z); };
func();
printPropDiff();
// Test that if an inner function has no _Varmap, it can still access
// the outer function's varmap.
print('- inner function without varmap, access outer function variables');
func = function foo(a,b) { var z = 123; function inner() { print(z); } inner(); };
func();
printPropDiff();
func = function foo() { var z = 123; function inner() { print(z); } inner(); };
func();
printPropDiff();
}
try {
test();
} catch (e) {
print(e.stack || e);
}

10
tests/ecmascript/util-object.js

@ -0,0 +1,10 @@
/* Get object property count using Duktape.info(). */
function getObjectPropertyCount(obj) {
var i = Duktape.info(obj);
if (typeof i === 'object' && 6 in i) {
return i[6];
} else if (typeof i === 'object' && 'enext' in i) {
return i.enext;
}
throw new Error('getObjectPropertyCount() relies on Duktape.info(), cannot parse result');
}

57
tests/knownissues/test-dev-func-varmap-drop-1.txt

@ -0,0 +1,57 @@
summary: debugger support enabled; affects _Varmap behavior
---
- pr(arg)
hello
prop count diff: 1
- Math.PI
3.141592653589793
prop count diff: 1
3.141592653589793
prop count diff: 0
- direct eval
3.141592653589793
prop count diff: 1
3.141592653589793
prop count diff: 0
- inner function
3.141592653589793
prop count diff: 1
3.141592653589793
prop count diff: 1
- try-catch without a slow path access in catch clause
3.141592653589793
prop count diff: 1
3.141592653589793
prop count diff: 0
- try-catch with a slow path access in catch clause
3.141592653589793
prop count diff: 1
3.141592653589793
prop count diff: 0
- with statement without a slow path access
3.141592653589793
prop count diff: 1
3.141592653589793
prop count diff: 0
- with statement with a slow path access
3.141592653589793
prop count diff: 1
3.141592653589793
prop count diff: 0
- arguments object access
3.141592653589793
object
prop count diff: 1
3.141592653589793
object
prop count diff: 0
- test eval creating a local variable for a function without _Varmap
123
prop count diff: 1
123
prop count diff: 0
- inner function without varmap, access outer function variables
123
prop count diff: 1
123
prop count diff: 1
Loading…
Cancel
Save