Thanks, here we go again. ;)
Performance: Let's not optimize this scenario to death unless we have a
real-world workload that needs to be considered. I agree that it's
unfortunate
that we have different implementations for tracking external strings and
external arraybuffers though. You are right about lookups and clears, but I
don't think that it's currently a bottle-neck. If you feel that this is
really
important and have a real-world workload I am open for discussion. We should
also include e.g. jochen@ in this case as he knows a lot more about other
embedders (chrome).
Refactoring: We try really hard to get our Heap properly encapsulated (still
work in progress) and offer a basic heap api which requires only including
src/heap/heap.h. We don't want the users of heap, i.e., general v8 api
(api.cc),
other runtime parts (object.cc, runtime-typed-arrays.cc) to know about how
we
track arraybuffers. If they register/unregister properly than they should be
fine. The comments below address these issues.
Testing: I think you should be able to write a cctest (in
test/cctest/test-heap.cc for example) which basically creates array buffers,
calls the GC (scavenge, full) and tracks
amount_of_externally_allocated_memory_
along the way.
https://codereview.chromium.org/1324023007/diff/20001/src/DEPS
File src/DEPS (right):
https://codereview.chromium.org/1324023007/diff/20001/src/DEPS#newcode8
src/DEPS:8: "+src/heap/array-buffer-tracker.h",
Remove this one.
https://codereview.chromium.org/1324023007/diff/20001/src/api.cc
File src/api.cc (right):
https://codereview.chromium.org/1324023007/diff/20001/src/api.cc#newcode35
src/api.cc:35: #include "src/heap/array-buffer-tracker.h"
Include only src/heap/heap.h
https://codereview.chromium.org/1324023007/diff/20001/src/api.cc#newcode6513
src/api.cc:6513:
isolate->heap()->array_buffer_tracker()->Unregister(*self);
Use the wrapper call for Unregister.
https://codereview.chromium.org/1324023007/diff/20001/src/api.cc#newcode6722
src/api.cc:6722:
isolate->heap()->array_buffer_tracker()->Unregister(*self);
Ditto.
https://codereview.chromium.org/1324023007/diff/20001/src/heap/heap.cc
File src/heap/heap.cc (right):
https://codereview.chromium.org/1324023007/diff/20001/src/heap/heap.cc#newcode168
src/heap/heap.cc:168: array_buffer_tracker_ = new
ArrayBufferTracker(this);
Move this into SetUp() somewhere next to ObjectStats.
The reason we do it this (in a separate SetUp call) way is that
initializer order (= declaration order) does -- unfortunately -- not
match with the order we need for some components.
https://codereview.chromium.org/1324023007/diff/20001/src/heap/heap.h
File src/heap/heap.h (left):
https://codereview.chromium.org/1324023007/diff/20001/src/heap/heap.h#oldcode1016
src/heap/heap.h:1016: void RegisterNewArrayBuffer(bool in_new_space,
void* data, size_t length);
Create public wrapper calls only for RegisterNewArrayBuffer and
UnregisterNewArrayBuffer in Heap. These calls should forward and provide
the interface into the heap.
https://codereview.chromium.org/1324023007/diff/20001/src/objects.cc
File src/objects.cc (right):
https://codereview.chromium.org/1324023007/diff/20001/src/objects.cc#newcode30
src/objects.cc:30: #include "src/heap/array-buffer-tracker.h"
Only include src/heap/heap.h
https://codereview.chromium.org/1324023007/diff/20001/src/objects.cc#newcode15947
src/objects.cc:15947:
isolate->heap()->array_buffer_tracker()->RegisterNew(
Use the wrapper call for Register.
https://codereview.chromium.org/1324023007/diff/20001/src/objects.cc#newcode16002
src/objects.cc:16002:
isolate->heap()->array_buffer_tracker()->RegisterNew(
Ditto.
https://codereview.chromium.org/1324023007/diff/20001/src/runtime/runtime-typedarray.cc
File src/runtime/runtime-typedarray.cc (right):
https://codereview.chromium.org/1324023007/diff/20001/src/runtime/runtime-typedarray.cc#newcode9
src/runtime/runtime-typedarray.cc:9: #include
"src/heap/array-buffer-tracker.h"
Only include src/heap/heap.h
https://codereview.chromium.org/1324023007/diff/20001/src/runtime/runtime-typedarray.cc#newcode96
src/runtime/runtime-typedarray.cc:96:
isolate->heap()->array_buffer_tracker()->Unregister(*array_buffer);
Use the wrapper call for Unregister.
https://codereview.chromium.org/1324023007/
--
--
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.
