http://codereview.chromium.org/10918067/diff/1/src/serialize.cc File src/serialize.cc (right):
http://codereview.chromium.org/10918067/diff/1/src/serialize.cc#newcode611 src/serialize.cc:611: isolate_->heap()->ReserveSpace(&reservations_[0], &high_water_[0]); On 2012/09/13 08:47:59, Yang wrote:
why dereference and re-reference, and not simply:
ReserveSpace(reservations_, high_water_)
unless you want to make sure it is obvious that we are dealing with
arrays? Done. http://codereview.chromium.org/10918067/diff/1/src/serialize.cc#newcode644 src/serialize.cc:644: isolate_->heap()->ReserveSpace(&reservations_[0], &high_water_[0]); On 2012/09/13 08:47:59, Yang wrote:
Ditto.
Ditto. http://codereview.chromium.org/10918067/diff/1/src/serialize.cc#newcode750 src/serialize.cc:750: current) + skip); \ On 2012/09/13 08:47:59, Yang wrote:
It seems easier readable to use Address instead of intptr_t.
Done. http://codereview.chromium.org/10918067/diff/1/src/serialize.cc#newcode762 src/serialize.cc:762: reinterpret_cast<intptr_t>(current) + skip); \ On 2012/09/13 08:47:59, Yang wrote:
Ditto.
Ditto http://codereview.chromium.org/10918067/diff/1/src/serialize.cc#newcode1000 src/serialize.cc:1000: ASSERT(integer <= 0x3fffff); On 2012/09/13 08:47:59, Yang wrote:
I'd prefer integer < (1 << 22)
Done. http://codereview.chromium.org/10918067/diff/1/src/serialize.cc#newcode1065 src/serialize.cc:1065: sink_->Put(kRawData + kPointerSize / 4, "Smi"); On 2012/09/13 08:47:59, Yang wrote:
The 4 in here obviously refers to the factor 4 in COMMON_RAW_LENGTH.
Can we
somehow make a constant out of this?
Now the 4 is gone. http://codereview.chromium.org/10918067/diff/1/src/serialize.h File src/serialize.h (right): http://codereview.chromium.org/10918067/diff/1/src/serialize.h#newcode177 src/serialize.h:177: answer = *reinterpret_cast<const int32_t*>(data_ + position_); On 2012/09/13 08:47:59, Yang wrote:
Since GetInt() uses this: we assume the least significant two bits to
tell how
many bytes are part of the integer, so the byte order matters. This
would only
work on little endian systems, right? The #else block seems to work
for big
endian as well. I don't think we have any big endian system that reads unaligned, but an assertion won't hurt?
Fixed to go into the slow case for big endian. http://codereview.chromium.org/10918067/diff/1/src/serialize.h#newcode182 src/serialize.h:182: answer |= data_[position_ + 3] << 24; On 2012/09/13 08:47:59, Yang wrote:
In PutInt we only take integers that can be encoded in 3 bytes. Why
even fetch a
4th byte?
Otherwise it will do something different depending on the ifdefs. I don't want to rename it to GetUnalignedIntOrGetUnaligned24BitValueDependingOnSomeObscureIfdefs because then I would run into problems with the 80 character limit. http://codereview.chromium.org/10918067/diff/1/src/serialize.h#newcode235 src/serialize.h:235: f(31, 124) On 2012/09/13 08:47:59, Yang wrote:
if the second arg is always 4x the first arg, why not make that part
of f? Or at
least add a few STATIC_CHECK in some of the f.
Made it part of f, and moved this to the .cc file. http://codereview.chromium.org/10918067/diff/1/src/serialize.h#newcode546 src/serialize.h:546: code_object_ = o->IsCode(); On 2012/09/13 08:47:59, Yang wrote:
I guess this could also be put into the initialization list.
Done. http://codereview.chromium.org/10918067/diff/1/src/snapshot-common.cc File src/snapshot-common.cc (right): http://codereview.chromium.org/10918067/diff/1/src/snapshot-common.cc#newcode92 src/snapshot-common.cc:92: bool success = V8::Initialize(&deserializer); On 2012/09/13 08:47:59, Yang wrote:
Is there any plans to include a SnapshotByteSink that writes to file
in
mksnapshot so that it can be read here?
This is currently only used for tests. http://codereview.chromium.org/10918067/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
