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

Issue 43001: don't output duplicate filenames in PDBSourceLineWriter (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
15 years ago by Ted Mielczarek
Modified:
15 years ago
Reviewers:
Mark Mentovai
Base URL:
http://google-breakpad.googlecode.com/svn/trunk/
Visibility:
Public.

Description

PDBSourceLineWriter currently can output duplicate filenames, since it simply
dumps the filenames + IDs given to it by the DIA APIs, and that data can contain
many duplicate filenames with different IDs. This patch caches filenames with
the first ID seen for them, and maps other IDs from duplicate filenames back to
the first.

I noticed this while investigating other PDBSourceLineWriter behavior. This
lowers the number of FILE lines in the symbol dump from our xul.pdb by almost
300k(!).

Mark, can you take a look at this?

Patch Set 1 #

Total comments: 7
Unified diffs Side-by-side diffs Delta from patch set Stats Patch
src/common/windows/pdb_source_line_writer.cc View 2 chunks +13 lines, -1 line 3 comments Download
src/common/windows/pdb_source_line_writer.h View 4 chunks +40 lines, -0 lines 4 comments Download

Messages

Total messages: 3
Ted Mielczarek
15 years ago #1
Ted Mielczarek
I forgot to mention, but I wrote a little script to verify the output of ...
15 years ago #2
Mark Mentovai
15 years ago #3
lg

http://breakpad.appspot.com/43001/diff/1/2
File src/common/windows/pdb_source_line_writer.cc (right):

http://breakpad.appspot.com/43001/diff/1/2#newcode121
Line 121: DWORD source_id;
I would rename this one to dia_source_id.

http://breakpad.appspot.com/43001/diff/1/2#newcode127
Line 127: source_id = GetRealFileID(source_id);
so that you have access to both source_id and dia_source_id.

http://breakpad.appspot.com/43001/diff/1/2#newcode226
Line 226: }
} else {

(even though I think it's yucky too.)

http://breakpad.appspot.com/43001/diff/1/3
File src/common/windows/pdb_source_line_writer.h (right):

http://breakpad.appspot.com/43001/diff/1/3#newcode39
Line 39: #include <hash_map>
Sort: h before s.

http://breakpad.appspot.com/43001/diff/1/3#newcode144
Line 144: bool FileIsCached(const wstring &file) {
The name FileIsCached doesn't really tell me that it's used to handle IDs and
that sort of thing.

http://breakpad.appspot.com/43001/diff/1/3#newcode155
Line 155: hash_map<wstring,DWORD>::iterator iter = unique_files_.find(file);
Space after comma, here and on 167.

http://breakpad.appspot.com/43001/diff/1/3#newcode195
Line 195: hash_map<DWORD,DWORD> file_ids_;
And here and on 197.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 1004:630ec63f810e-tainted