Make snapshots reproducible.

Make snapshots reproducible.

To keep the structure of the serializer more or less untouched, we use
some ingenious Corry-approved(TM) 3-step technology (a.k.a. "hack"):

   * Create copies of code objects.
   * Wipe out all absolute addresses in these copies.
   * Write out the cleaned copies instead of the originals.

In conjunction with --random-seed, our snapshots are reproducible now.

BUG=v8:2885
R=bmeurer@chromium.org, erik.corry@gmail.com

Committed: http://code.google.com/p/v8/source/detail?r=17473

[Benedikt Meurer, 2013-11-04 11:25:14]

LGTM with nit

https://codereview.chromium.org/54823002/diff/40001/src/serialize.cc
File src/serialize.cc (right):

https://codereview.chromium.org/54823002/diff/40001/src/serialize.cc#newcode1787
src/serialize.cc:1787: code->set_relocation_info(
Nit: Can we do better here?

[Sven Panne, 2013-11-04 12:24:25]

Slightly polished, PTAL again

[Benedikt Meurer, 2013-11-04 12:25:45]

On 2013/11/04 12:24:25, Sven Panne wrote:
> Slightly polished, PTAL again

LGTM

[Erik Corry, 2013-11-04 12:41:13]

lgtm

https://codereview.chromium.org/54823002/diff/90001/src/ia32/assembler-ia32-i...
File src/ia32/assembler-ia32-inl.h (right):

https://codereview.chromium.org/54823002/diff/90001/src/ia32/assembler-ia32-i...
src/ia32/assembler-ia32-inl.h:256: Assembler::set_target_address_at(pc_, pc_ +
sizeof(int32_t));
Why add sizeof(int32_t) here?

https://codereview.chromium.org/54823002/diff/90001/src/objects.h
File src/objects.h (right):

https://codereview.chromium.org/54823002/diff/90001/src/objects.h#newcode5199
src/objects.h:5199: // Slightly ugly, used in ObjectSerializer only.
Rather than insulting your own code it would be more helpful to spend a few
words explaining why it's needed.

[Sven Panne, 2013-11-04 13:03:19]

https://codereview.chromium.org/54823002/diff/90001/src/ia32/assembler-ia32-i...
File src/ia32/assembler-ia32-inl.h (right):

https://codereview.chromium.org/54823002/diff/90001/src/ia32/assembler-ia32-i...
src/ia32/assembler-ia32-inl.h:256: Assembler::set_target_address_at(pc_, pc_ +
sizeof(int32_t));
On 2013/11/04 12:41:14, Erik Corry wrote:
> Why add sizeof(int32_t) here?

Because our Intel assemblers subtract this, but it is not strictly necessary,
any constant offset would do. I just wanted to effectively write zero at the
relocation.

https://codereview.chromium.org/54823002/diff/90001/src/objects.h
File src/objects.h (right):

https://codereview.chromium.org/54823002/diff/90001/src/objects.h#newcode5199
src/objects.h:5199: // Slightly ugly, used in ObjectSerializer only.
On 2013/11/04 12:41:14, Erik Corry wrote:
> Rather than insulting your own code it would be more helpful to spend a few
> words explaining why it's needed.

Well, that would just be yet another general v8 comment: What we are lacking
here again is a clear distinction between raw data structure manipulation and
some semantics layered upon that. I just want the "raw stuff" here.

Furthermore, from a more conceptual POV this doesn't really belong into the Code
class at all, it just lives here because we suffer from the common OOP problem
of the "tyranny of the hierarchy". Duplicating READ_FIELD/WRITE_FIELD in the
serializer would have been worse.

I am not sure how to put that into a non-insulting but enlightening comment...
:-)

[Erik Corry, 2013-11-04 13:20:02]

https://codereview.chromium.org/54823002/diff/90001/src/objects.h
File src/objects.h (right):

https://codereview.chromium.org/54823002/diff/90001/src/objects.h#newcode5199
src/objects.h:5199: // Slightly ugly, used in ObjectSerializer only.
On 2013/11/04 13:03:19, Sven Panne wrote:
> On 2013/11/04 12:41:14, Erik Corry wrote:
> > Rather than insulting your own code it would be more helpful to spend a few
> > words explaining why it's needed.
> 
> Well, that would just be yet another general v8 comment: What we are lacking
> here again is a clear distinction between raw data structure manipulation and
> some semantics layered upon that. I just want the "raw stuff" here.
> 
> Furthermore, from a more conceptual POV this doesn't really belong into the
Code
> class at all, it just lives here because we suffer from the common OOP problem
> of the "tyranny of the hierarchy". Duplicating READ_FIELD/WRITE_FIELD in the
> serializer would have been worse.
> 
> I am not sure how to put that into a non-insulting but enlightening comment...
> :-)

Let me suggest:

"The entire code object including its header is copied verbatim to the snapshot
so that it can be written in one, fast, memcpy during deserialization.  The
deserializer will overwrite some pointers, rather like a runtime linker, but the
random allocation addresses used in the mksnapshot process would still be
present in the unlinked snapshot data, which would make snapshot production
non-reproducible.  This method wipes out the to-be-overwritten header data for
reproducible snapshots."

[Sven Panne, 2013-11-04 13:35:34]

Added that + 2 tiny comments about the sizeof(int32_t). Thanks for ghost
writing! :-D

[Sven Panne, 2013-11-05 10:14:59]

Committed patchset #5 manually as r17473 (presubmit successful).