Please also run out performance regression tests to make sure this is performance neutral.
http://chromiumcodereview.appspot.com/10448007/diff/10001/src/splay-tree-inl.h File src/splay-tree-inl.h (right): http://chromiumcodereview.appspot.com/10448007/diff/10001/src/splay-tree-inl.h#newcode189 src/splay-tree-inl.h:189: deallocator_.Delete(node_to_move); Yikes! You never call the node's destructor this way. I think you need to have the node contain the deallocator, too, for symmetry. http://chromiumcodereview.appspot.com/10448007/diff/10001/src/splay-tree.h File src/splay-tree.h (right): http://chromiumcodereview.appspot.com/10448007/diff/10001/src/splay-tree.h#newcode127 src/splay-tree.h:127: UNREACHABLE(); Hmmm. This still seems dangerous. It seems to introduce unnecessary asymmetry. http://chromiumcodereview.appspot.com/10448007/diff/10001/src/splay-tree.h#newcode141 src/splay-tree.h:141: Node* right_; I think the cleanest way is to make sure Deallocator is a member here. http://chromiumcodereview.appspot.com/10448007/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
