http://codereview.chromium.org/8917014/diff/3/src/deoptimizer.cc File src/deoptimizer.cc (right):
http://codereview.chromium.org/8917014/diff/3/src/deoptimizer.cc#newcode269 src/deoptimizer.cc:269: Object* global = Context::cast(context)->get(Context::GLOBAL_INDEX); On 2011/12/12 19:11:16, ulan wrote:
Do we need check here? Is global object always defined?
I think we need it. Judging from the control flow inside Genesis::Genesis we can get GC before set_global was called. Please add a comment about this here (so people reading the code will know that GC can happen when context is in the list but not yet fully initialized). http://codereview.chromium.org/8917014/diff/3/src/heap.cc File src/heap.cc (right): http://codereview.chromium.org/8917014/diff/3/src/heap.cc#newcode647 src/heap.cc:647: Context::cast(context)->jsfunction_result_caches(); Should not you guard here instead of guarding below? (set_jsfunction_result_caches is called once the cache is fully constructed). also please comment why we need to guard against undefined. http://codereview.chromium.org/8917014/diff/3/src/heap.cc#newcode671 src/heap.cc:671: int index = Context::NORMALIZED_MAP_CACHE_INDEX; I don't think we need separate variable just for index. http://codereview.chromium.org/8917014/diff/3/src/heap.cc#newcode673 src/heap.cc:673: if (!cache->IsUndefined()) { please comment why we need to guard against partial initialized objects. http://codereview.chromium.org/8917014/diff/3/src/incremental-marking.cc File src/incremental-marking.cc (right): http://codereview.chromium.org/8917014/diff/3/src/incremental-marking.cc#newcode680 src/incremental-marking.cc:680: int index = Context::NORMALIZED_MAP_CACHE_INDEX; I don't think we need separate index variable. http://codereview.chromium.org/8917014/diff/3/src/incremental-marking.cc#newcode682 src/incremental-marking.cc:682: if (!cache_or_undefined->IsUndefined()) { Please comment why can it be undefined. http://codereview.chromium.org/8917014/diff/3/src/incremental-marking.cc#newcode683 src/incremental-marking.cc:683: NormalizedMapCache* cache = NormalizedMapCache::cast(cache_or_undefined); you don't need a separate cache variable. just make cache_or_undefined cache. (make it HeapObject as it is guaranteed to be one). http://codereview.chromium.org/8917014/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
