I intentionally commented on the first patch-set, because it contains both
affected files. My proposal is to delay the construction of the StubCache a
bit,
and then do it in a single step via the constructor. This way we avoid
having a
dangerous half-initialized object lying around, something which is always
worthwhile.
If it turns out that we need this 2-step construction, we can at least nuke
the
boolean argument to Initialize.
http://codereview.chromium.org/9464054/diff/1/src/isolate.cc
File src/isolate.cc (right):
http://codereview.chromium.org/9464054/diff/1/src/isolate.cc#newcode1633
src/isolate.cc:1633: free(stub_cache_);
Leave this as-is.
http://codereview.chromium.org/9464054/diff/1/src/isolate.cc#newcode1782
src/isolate.cc:1782: stub_cache_ = new (calloc(1, sizeof(StubCache)))
StubCache(this);
I think we can remove this line completely and replace it with something
below...
http://codereview.chromium.org/9464054/diff/1/src/isolate.cc#newcode1839
src/isolate.cc:1839: stub_cache_->Initialize(create_heap_objects);
Remove this line.
http://codereview.chromium.org/9464054/diff/1/src/isolate.cc#newcode1844
src/isolate.cc:1844: stub_cache_->Initialize(true);
I think we can move this Initialize past the condition and replace it
with a "stub_cache_ = new StubCache(this);" there, because I doubt that
the uninitialized StubCache can be used before this point, anyway.
http://codereview.chromium.org/9464054/diff/1/src/stub-cache.cc
File src/stub-cache.cc (left):
http://codereview.chromium.org/9464054/diff/1/src/stub-cache.cc#oldcode46
src/stub-cache.cc:46: StubCache::StubCache(Isolate* isolate) :
isolate_(isolate) {
Merge the constructor with the Initialize function below, assuming
create_heap_objects == true. This way, we only construct a nice, clean
initialized object.
http://codereview.chromium.org/9464054/
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
