On 2012/04/10 13:50:13, Mikhail Naganov (Chromium) wrote:
The description isn't very clear: "This will give us the ability to keep
entries_ list in sorted by id state."
- what is "id state"? Did you mean simply "id"?
yep. I mean sorted by id
- not sure, how does adding the 'addr' field help to keep the list
sorted by
id. This is just happens because ids increase, and we add new entries to
the
tail of the list.
It was true only until the first call of RemoveDeadEntries
This method iterated over hash map and the new_entries list became unsorted.
Did you mean, that adding the 'addr' field will allow to do fast reverse
lookups
from id to address, having that 'entries_' list is naturally sorted by id?
At this moment we are using HeapSnaphot entries list that also is unsorted
and
we do sorting
specially for GetNodeById.
https://chromiumcodereview.appspot.com/10031032/diff/1/src/profile-generator.cc
File src/profile-generator.cc (right):
https://chromiumcodereview.appspot.com/10031032/diff/1/src/profile-generator.cc#newcode1345
src/profile-generator.cc:1345: int
HeapObjectsMap::UpdateEntryInfoAddress(Address addr,
I don't see a clear reason for having EntryInfo lookup and address update
in a
single function. I think, restricting the scope to only making a lookup
will
make it more clean. Also this should simplify logic in MoveObject (see
below).
It is a bit complex thing.
I have to update address in From EntryInfo because the object was moved.
Also I have to clean address in To EntryInfo because I use it
RemoveDeadEntries
method.
If I have skipped the last step I would have removed this To address from
the
entries_map because
To EntryInfo hasn't been marked as accessed.
https://chromiumcodereview.appspot.com/10031032/diff/1/src/profile-generator.cc#newcode1372
src/profile-generator.cc:1372: UpdateEntryInfoAddress(to, to_hash, NULL);
Why are you making 2 map lookups here?
I can't avoid the first lookup for From address because I have no Remove
method
which returns the value of removed entry.
I can't avoid second lookup for To address because I can't distinguish the
case
when I got a new entry with NULL as the value and the case when I got an old
entry with 0 as entry_index.
I've plan to fix both the cases later.
https://chromiumcodereview.appspot.com/10031032/diff/1/src/profile-generator.h
File src/profile-generator.h (right):
https://chromiumcodereview.appspot.com/10031032/diff/1/src/profile-generator.h#newcode736
src/profile-generator.h:736: int UpdateEntryInfoAddress(Address addr,
uint32_t
hash, Address new_addr);
hash -> addr_hash. Otherwise, it's not clear where it belongs to.
https://chromiumcodereview.appspot.com/10031032/
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
