https://codereview.chromium.org/1105693002/diff/1/src/heap/identity-map.cc
File src/heap/identity-map.cc (right):
https://codereview.chromium.org/1105693002/diff/1/src/heap/identity-map.cc#newcode17
src/heap/identity-map.cc:17: if (concurrent_)
heap_->relocation_mutex()->Lock();
How about a simple helper class like this:
namespace {
class AutoLocker {
public:
AutoLocker(Heap* heap, bool concurrent) : heap_(heap),
concurrent_(concurent) {
if (concurrent_) heap_->relocation_mutex()->Lock();
}
~AutoLocker() { if (concurrent_) heap_->relocation_mutex()->Unlock();
}
private:
Heap* heap_;
bool concurrent_;
};
}
and then use it like this:
AutoLocker locker(heap_, concurrent_);
same below. That should make the code more robust.
https://codereview.chromium.org/1105693002/diff/1/src/heap/identity-map.h
File src/heap/identity-map.h (right):
https://codereview.chromium.org/1105693002/diff/1/src/heap/identity-map.h#newcode51
src/heap/identity-map.h:51: RawEntry Lookup(Handle<Object> handle) {
Style nit: Since these methods are private, and all call sites are in
the .cc file anyway, there's probably no reason to have them in the .h
file. Can you move them to the .cc file as well?
https://codereview.chromium.org/1105693002/diff/1/src/heap/identity-map.h#newcode53
src/heap/identity-map.h:53: int index = LookupIndex(*handle);
How about size_t here, and check for index < size_ instead of index >=
0?
https://codereview.chromium.org/1105693002/diff/1/src/heap/identity-map.h#newcode57
src/heap/identity-map.h:57: RawEntry Insert(Handle<Object> handle) {
Style nit: Since these methods are private, and all call sites are in
the .cc file anyway, there's probably no reason to have them in the .h
file. Can you move them to the .cc file as well?
https://codereview.chromium.org/1105693002/diff/1/src/heap/identity-map.h#newcode64
src/heap/identity-map.h:64: int Hash(Object* address) {
Style nit: Since these methods are private, and all call sites are in
the .cc file anyway, there's probably no reason to have them in the .h
file. Can you move them to the .cc file as well?
https://codereview.chromium.org/1105693002/diff/1/src/heap/identity-map.h#newcode68
src/heap/identity-map.h:68: return static_cast<int>((raw_address >> 11)
^ raw_address);
Down casting to int looks wrong here. How about using uintptr_t here? Or
even better use size_t?
https://codereview.chromium.org/1105693002/
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
