Addressed comments.
Compiling with external snapshot uncovered some compilation issues. Test
suite
fails though, with and without this patch. I assume it's because the test
harness does not pass the correct blob files to d8?
https://codereview.chromium.org/781443002/diff/1/src/serialize.cc
File src/serialize.cc (right):
https://codereview.chromium.org/781443002/diff/1/src/serialize.cc#newcode2313
src/serialize.cc:2313: data_ = NewArray<byte>(size_);
On 2014/12/03 18:49:25, vogelheim wrote:
Related to the comment in serialize.h:
I was trying to understand how the memory management works, and the
only place I
found where owns_data_ is set to true was in the superclass
constructor. So if I
get this right, this piece of code relies on:
- the super-class constructor creating an 'owned' NULL pointer
- then this allocates memory
- then the destructor of the super class de-allocates it.
I guess that works, but I for one had a hard time understanding it.
[Likewise for SerializedCodeData below.]
I moved allocation into a helper method in the parent class, which also
sets owns_data_.
https://codereview.chromium.org/781443002/diff/1/src/serialize.cc#newcode2408
src/serialize.cc:2408: return result;
On 2014/12/03 18:49:25, vogelheim wrote:
Is this expected to the last method call on a particular
SerializedCodeData
instance? In that case, maybe also null-ify the data pointer when
releasing
ownership.
Done.
https://codereview.chromium.org/781443002/diff/1/src/serialize.h
File src/serialize.h (right):
https://codereview.chromium.org/781443002/diff/1/src/serialize.h#newcode411
src/serialize.h:411: SerializedData() : data_(NULL), size_(0),
owns_data_(true) {}
On 2014/12/03 18:49:25, vogelheim wrote:
Here, owns_data_ == (data_ == NULL), which doesn't seem to make sense.
If I got this right, several subclass constructors will put 'owned'
data_ there
by allocating memory. In that case it may make more sense to also let
those
subclasses delete that memory in their destructors.
Your choice, though...
Acknowledged.
https://codereview.chromium.org/781443002/
--
--
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.
