Thanks for all the feedback; I've uploaded a new revision.
https://chromiumcodereview.appspot.com/9221011/diff/4001/src/ast.h File src/ast.h (right): https://chromiumcodereview.appspot.com/9221011/diff/4001/src/ast.h#newcode1254 src/ast.h:1254: AstProperties* ast_properties_; On 2012/01/30 13:54:26, fschneider wrote:
It seems quite expensive to add a pointer to each VariableProxy to
record later
changes in the AstProperties. VariableProxy is one of the most
frequent of all
AST nodes.
Maybe pass the AstProperties instead on to the phase that may change
the
VariableProxy?
The problem is that at the time the VariableProxy is bound to a variable (via BindTo()), the kDontInline bit potentially needs to be flipped. I don't see a way to do that without either storing this pointer (which costs memory, yes) or walking the tree top-down to find all VariableProxies (which costs time). If you have any other solution in mind, I'm all ears :-) https://chromiumcodereview.appspot.com/9221011/diff/4001/src/ast.h#newcode2349 src/ast.h:2349: class AstNodeFactory { On 2012/01/30 12:10:35, fschneider wrote:
maybe make it BASE_EMBEDDED.
Done. https://chromiumcodereview.appspot.com/9221011/diff/4001/src/objects.h File src/objects.h (right): https://chromiumcodereview.appspot.com/9221011/diff/4001/src/objects.h#newcode5412 src/objects.h:5412: kAstNodeCountOffset + kPointerSize; On 2012/01/30 13:54:26, fschneider wrote:
You could reduce the x64 version by pairing two properties from above
like done
for the ones below here.
Done. https://chromiumcodereview.appspot.com/9221011/diff/4001/src/parser.h File src/parser.h (right): https://chromiumcodereview.appspot.com/9221011/diff/4001/src/parser.h#newcode797 src/parser.h:797: AstNodeFactory* ast_node_factory_; On 2012/01/30 12:10:35, fschneider wrote:
Embedded instance
AstNodeFactory ast_node_factory_;
instead of malloced with new/delete would be better.
Done. https://chromiumcodereview.appspot.com/9221011/diff/4001/src/rewriter.cc File src/rewriter.cc (right): https://chromiumcodereview.appspot.com/9221011/diff/4001/src/rewriter.cc#newcode46 src/rewriter.cc:46: factory_ = new AstNodeFactory(isolate()); On 2012/01/30 12:10:35, fschneider wrote:
Why not just embed it in the instance? Then there is no need to call
new/delete
(which is expensive if it is malloced)
Done. https://chromiumcodereview.appspot.com/9221011/diff/4001/src/rewriter.cc#newcode57 src/rewriter.cc:57: return factory_; On 2012/01/30 12:10:35, fschneider wrote:
return &factory_;
Done. https://chromiumcodereview.appspot.com/9221011/diff/4001/src/rewriter.cc#newcode76 src/rewriter.cc:76: AstNodeFactory* factory_; On 2012/01/30 12:10:35, fschneider wrote:
AstNodeFactory factory_;
Done. https://chromiumcodereview.appspot.com/9221011/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
