More comments to follow in personal discussion. -ip
http://codereview.chromium.org/7990/diff/401/603 File src/runtime.cc (right): http://codereview.chromium.org/7990/diff/401/603#newcode3996 Line 3996: * whether the storage is a fixed array or storage. Should probably read: The second parameter of the constructor, fast_elements, specifies whether the storage is a FixedArray or a Dictionary. http://codereview.chromium.org/7990/diff/401/603#newcode4018 Line 4018: bool fast_elements_; Maybe if you would put the base offset into this visitor then you would not have to keep passing it around in the iterators below? http://codereview.chromium.org/7990/diff/401/603#newcode4034 Line 4034: uint32_t visitor_index_offset) { See comment above about not passing the visitor_index_offset around all the time... http://codereview.chromium.org/7990/diff/401/603#newcode4122 Line 4122: * as it is an one-element array. as it -> as if it http://codereview.chromium.org/7990/diff/401/603#newcode4137 Line 4137: // Avoid obvious duplicated elements. Please explain how one could get duplicated elements. My guess is that the double counting for elements in the prototype chain could have accounted for more elements than the length itself, correct? http://codereview.chromium.org/7990/diff/401/603#newcode4184 Line 4184: bool fast_case = estimate_nof_elements * 2 >= result_length; Please use parens around the arithmetic before the comparison. It makes it much more readable in my opinion. http://codereview.chromium.org/7990 --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
