http://codereview.chromium.org/9634005/diff/7002/src/spaces.cc
File src/spaces.cc (right):

http://codereview.chromium.org/9634005/diff/7002/src/spaces.cc#newcode2616
src/spaces.cc:2616: LargePage* page = FindPageContainingPc(a);
On 2012/03/12 02:00:40, kewpie.w.zp wrote:
On 2012/03/09 12:15:37, Vyacheslav Egorov wrote:
> I would suggest retaining FindPageContainingPc because you are
calling it from
> the function that is used for arbitrary addresses.

Not very sure if I catch your point. Please allow me to confirm. Do
you suggest
retaining the linear search of FindPageContainingPc() and just change
FindObject() to hash lookup, or reversed, or something else ? Thanks a
lot for
your answering

My original comment should read: "I would suggest _renaming_...". Sorry
for the confusion, there was a typo.

My point was that there is nothing PC or code object specific in the
code.

http://codereview.chromium.org/9634005/diff/7002/src/spaces.cc#newcode2633
src/spaces.cc:2633: if (page_address <= pc && pc < page_address +
page->size()) {
On 2012/03/12 02:00:40, kewpie.w.zp wrote:
On 2012/03/09 12:15:37, Vyacheslav Egorov wrote:
> Consider using page->Contains(pc) here.

The check range of page->Contains(pc) falls into
(base+Page::kObjectStartOffset,
base+chunk_size), so if pc falls into page's header range (base,
base+Page::kObjectStartOffset) will be failed, which would succeed
otherwise.
Should we allow this difference and replace it with Contains() anyway
?

I don't think anybody should pass addresses that fall outside of inner
object area to this function. If anybody does --- that is clearly a bug.

http://codereview.chromium.org/9634005/

--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to