Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(706)

Issue 4802056: Make Decomposer::DecomposedImage serializable. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
12 years, 8 months ago by chrisha
Modified:
12 years, 8 months ago
Reviewers:
Roger McFarlane, Siggi
CC:
sawbuck-changes_googlegroups.com
Base URL:
http://sawbuck.googlecode.com/svn/trunk/
Visibility:
Public.

Description

Make Decomposer::DecomposedImage serializable. Adds serialization to Decomposer::DecomposedImage. Since this is a 'deep' data structure with pointers flying everywhere, this involves some serialization trickery, and the addition of serialization to many basic types (BlockGraph, AddressSpace, etc). Loading a BlockGraph from its serialized form is 5x faster than redecomposing the entire thing from a warm PDB file. Creates 'decompose', a new tool for decomposing an image and serializing its BlockGraph representation. The aim will be to make each tool in the toolchain optionally load a block graph directly, rather than redecomposing. The serialized format contains metadata for validating that it has been generated with a compatible version of the toolchain. Split core/unittest_util.* into a new static library so that the pe unittests can also use the functionality present there. Added a bunch of unittests for the new functionality. Committed: http://code.google.com/p/sawbuck/source/browse/#svn/trunk393

Patch Set 1 : '' #

Patch Set 2 : '' #

Total comments: 50

Patch Set 3 : '' #

Total comments: 5

Patch Set 4 : '' #

Unified diffs Side-by-side diffs Delta from patch set Stats (+1143 lines, -98 lines) Patch
M syzygy/common/syzygy_version.h View 1 1 chunk +2 lines, -1 line 0 comments Download
M syzygy/core/address.h View 1 2 chunks +3 lines, -2 lines 0 comments Download
M syzygy/core/address_space.h View 1 2 1 chunk +3 lines, -0 lines 0 comments Download
M syzygy/core/block_graph.h View 1 2 7 chunks +43 lines, -1 line 0 comments Download
M syzygy/core/block_graph.cc View 1 2 3 4 chunks +256 lines, -0 lines 0 comments Download
M syzygy/core/block_graph_unittest.cc View 1 2 chunks +49 lines, -0 lines 0 comments Download
M syzygy/core/core.gyp View 1 2 chunks +15 lines, -1 line 0 comments Download
M syzygy/core/serialization.h View 1 4 chunks +14 lines, -12 lines 0 comments Download
M syzygy/core/serialization.cc View 1 2 3 2 chunks +36 lines, -0 lines 0 comments Download
M syzygy/core/serialization_impl.h View 1 2 3 2 chunks +15 lines, -20 lines 0 comments Download
M syzygy/core/unittest_util.h View 1 2 chunks +10 lines, -0 lines 0 comments Download
A syzygy/core/unittest_util.cc View 1 2 1 chunk +104 lines, -0 lines 0 comments Download
A syzygy/pe/decompose_main.cc View 1 2 1 chunk +253 lines, -0 lines 0 comments Download
M syzygy/pe/decomposer.h View 1 3 chunks +16 lines, -0 lines 0 comments Download
M syzygy/pe/decomposer.cc View 1 2 6 chunks +212 lines, -18 lines 0 comments Download
M syzygy/pe/decomposer_unittest.cc View 1 2 chunks +43 lines, -1 line 0 comments Download
M syzygy/pe/metadata.h View 1 2 chunks +2 lines, -20 lines 0 comments Download
M syzygy/pe/metadata.cc View 1 1 chunk +18 lines, -0 lines 0 comments Download
M syzygy/pe/metadata_unittest.cc View 1 1 chunk +1 line, -1 line 0 comments Download
M syzygy/pe/pe.gyp View 1 2 chunks +20 lines, -0 lines 0 comments Download
M syzygy/pe/pe_file.h View 1 3 chunks +5 lines, -21 lines 0 comments Download
M syzygy/pe/pe_file.cc View 1 1 chunk +23 lines, -0 lines 0 comments Download

Messages

Total messages: 10
chrisha
PTAL.
12 years, 8 months ago (2011-07-22 20:18:54 UTC) #1
Siggi
This feels like an over-generalization - are there other instances were we need this? I'd ...
12 years, 8 months ago (2011-07-25 10:29:31 UTC) #2
chrisha
Yeah, in retrospect this is more of a 'solution looking for a problem'. I suppose ...
12 years, 8 months ago (2011-07-25 16:32:58 UTC) #3
chrisha
PTAL
12 years, 8 months ago (2011-08-02 20:52:13 UTC) #4
Siggi
nice! Here's my first batch of comments, more to come tomorrow morning. http://codereview.appspot.com/4802056/diff/10001/syzygy/core/address_space.h File syzygy/core/address_space.h ...
12 years, 8 months ago (2011-08-02 21:27:25 UTC) #5
Siggi
Here's the rest of my nits. http://codereview.appspot.com/4802056/diff/10001/syzygy/core/unittest_util.cc File syzygy/core/unittest_util.cc (right): http://codereview.appspot.com/4802056/diff/10001/syzygy/core/unittest_util.cc#newcode44 syzygy/core/unittest_util.cc:44: return false; nit: ...
12 years, 8 months ago (2011-08-03 13:06:33 UTC) #6
Roger McFarlane
http://codereview.appspot.com/4802056/diff/10001/syzygy/core/serialization.h File syzygy/core/serialization.h (right): http://codereview.appspot.com/4802056/diff/10001/syzygy/core/serialization.h#newcode264 syzygy/core/serialization.h:264: bool Save(const Type& x) { \ In order to ...
12 years, 8 months ago (2011-08-03 14:04:31 UTC) #7
chrisha
First batch of comments addressed. PTA(nother)L http://codereview.appspot.com/4802056/diff/10001/syzygy/core/address_space.h File syzygy/core/address_space.h (right): http://codereview.appspot.com/4802056/diff/10001/syzygy/core/address_space.h#newcode153 syzygy/core/address_space.h:153: bool Save(OutArchive* out_archive) ...
12 years, 8 months ago (2011-08-03 14:41:22 UTC) #8
Siggi
lgmt with a couple of nits. http://codereview.appspot.com/4802056/diff/19021/syzygy/core/block_graph.cc File syzygy/core/block_graph.cc (right): http://codereview.appspot.com/4802056/diff/19021/syzygy/core/block_graph.cc#newcode633 syzygy/core/block_graph.cc:633: Size num_refs = ...
12 years, 8 months ago (2011-08-03 15:06:34 UTC) #9
chrisha
12 years, 8 months ago (2011-08-03 15:29:12 UTC) #10
Thanks, committing.

http://codereview.appspot.com/4802056/diff/19021/syzygy/core/block_graph.cc
File syzygy/core/block_graph.cc (right):

http://codereview.appspot.com/4802056/diff/19021/syzygy/core/block_graph.cc#n...
syzygy/core/block_graph.cc:633: Size num_refs = 0;
On 2011/08/03 15:06:34, Siggi wrote:
> it's a little confusing to occlude the outer num_refs like this, maybe a
> different name for this?

Done.

http://codereview.appspot.com/4802056/diff/19021/syzygy/core/serialization_im...
File syzygy/core/serialization_impl.h (right):

http://codereview.appspot.com/4802056/diff/19021/syzygy/core/serialization_im...
syzygy/core/serialization_impl.h:30: struct _OMAP;
On 2011/08/03 15:06:34, Siggi wrote:
> also typedef to OMAP as we discussed.

Done.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b