This is a (first) MR about optmizing (new) PDB reader.
Context: - game ships with a 1+ GB file and throws an exception at some point in the game, then captures a backtrace with a bunch of dbghelp invocations. - exception is also raised on native but is handled rather quickly (1-2seconds wall time), during what the game completly freezes.
Old PDB reader (first context was on Wine 9.x) crashed when reading the PDB file. Serie starts with a fix for that, but it takes ages to load the PDB file. Context moved to Wine 10.x (with new PDB reader).
Freeze time is about 7-8s. So this serie and subsequent ones will reduce freeze time (current status is 2.6s).
This serie contains: - fix for old PDB reader to avoid crash, - some cleanups in PDB handling (nothing too fancy), - remove the computation of CRC when considering an ELF module, (the only place where this is used if when generating minidump including ELF modules, but the CRC is recomputed anyway). Since the CRC is computed on the whole ELF module image, and depending on number of referenced ELF modules, this can save quite some time (here ~1s).
From: Eric Pouech epouech@codeweavers.com
Signed-off-by: Eric Pouech epouech@codeweavers.com
(imported from commit 9a300765deded082b517551c60d28ed5267fecc7) --- dlls/dbghelp/msc.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/dlls/dbghelp/msc.c b/dlls/dbghelp/msc.c index f2a697e1056..1a75dab70ac 100644 --- a/dlls/dbghelp/msc.c +++ b/dlls/dbghelp/msc.c @@ -2714,10 +2714,11 @@ static BOOL codeview_snarf(const struct msc_debug_info* msc_dbg, } else { + const union codeview_symbol* end_sym; /* skip all records until paired S_INLINESITE_END */ - sym = (const union codeview_symbol*)(root + sym->inline_site_v3.pEnd); - if (sym->generic.id != S_INLINESITE_END) FIXME("complete wreckage\n"); - length = sym->inline_site_v3.pEnd - i + sym->generic.len; + end_sym = (const union codeview_symbol*)(root + sym->inline_site_v3.pEnd); + if (end_sym->generic.id != S_INLINESITE_END) FIXME("complete wreckage\n"); + length = sym->inline_site_v3.pEnd - i + end_sym->generic.len + 2; } } break; @@ -2736,10 +2737,11 @@ static BOOL codeview_snarf(const struct msc_debug_info* msc_dbg, } else { + const union codeview_symbol* end_sym; /* skip all records until paired S_INLINESITE_END */ - sym = (const union codeview_symbol*)(root + sym->inline_site2_v3.pEnd); - if (sym->generic.id != S_INLINESITE_END) FIXME("complete wreckage\n"); - length = sym->inline_site2_v3.pEnd - i + sym->generic.len; + end_sym = (const union codeview_symbol*)(root + sym->inline_site2_v3.pEnd); + if (end_sym->generic.id != S_INLINESITE_END) FIXME("complete wreckage\n"); + length = sym->inline_site2_v3.pEnd - i + end_sym->generic.len + 2; } } break;
From: Eric Pouech epouech@codeweavers.com
Signed-off-by: Eric Pouech epouech@codeweavers.com --- dlls/dbghelp/pdb.c | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/dlls/dbghelp/pdb.c b/dlls/dbghelp/pdb.c index 2ff1ca119cb..b2daa6bab0f 100644 --- a/dlls/dbghelp/pdb.c +++ b/dlls/dbghelp/pdb.c @@ -108,7 +108,8 @@ struct pdb_compiland { pdbsize_t stream_offset; /* in DBI stream for compiland description */ unsigned short are_symbols_loaded; - unsigned short stream_id; /* for all symbols of given compiland */ + unsigned short compiland_stream_id; + pdbsize_t compiland_symbols_size; struct symt_compiland* compiland; };
@@ -1902,8 +1903,9 @@ static enum pdb_result pdb_reader_init_DBI(struct pdb_reader *pdb) { pdb->compilands[i].stream_offset = compiland_iter.dbi_walker.offset; pdb->compilands[i].compiland = NULL; - pdb->compilands[i].stream_id = compiland_iter.dbi_cu_header.stream; - pdb->compilands[i].are_symbols_loaded = pdb->compilands[i].stream_id == 0xffff; + pdb->compilands[i].compiland_stream_id = compiland_iter.dbi_cu_header.stream; + pdb->compilands[i].compiland_symbols_size = compiland_iter.dbi_cu_header.symbol_size; + pdb->compilands[i].are_symbols_loaded = pdb->compilands[i].compiland_stream_id == 0xffff; result = pdb_reader_compiland_iterator_next(pdb, &compiland_iter); if ((result == R_PDB_SUCCESS) != (i + 1 < pdb->num_compilands)) return result ? result : R_PDB_INVALID_PDB_FILE; } @@ -4284,6 +4286,10 @@ static enum pdb_result pdb_reader_load_compiland_symbols(struct pdb_reader *pdb, unsigned top_frame_size = -1; DWORD64 address; symref_t type_symref; + + /* skip first DWORD, always 4 AFAICT */ + walker->offset += sizeof(UINT32); + /* * Loop over the different types of records and whenever we * find something we are interested in, record it and move on. @@ -4633,8 +4639,9 @@ static enum pdb_result pdb_reader_ensure_symbols_loaded_from_compiland(struct pd compiland->compiland = symt_new_compiland(pdb->module, obj_name); pdb_reader_free(pdb, obj_name);
- if ((result = pdb_reader_walker_init(pdb, compiland->stream_id, &walker))) return result; - walker.offset = sizeof(UINT32); + if ((result = pdb_reader_walker_init(pdb, compiland->compiland_stream_id, &walker))) return result; + if ((result = pdb_reader_walker_narrow(&walker, 0, compiland->compiland_symbols_size))) return result; + if ((result = pdb_reader_load_compiland_symbols(pdb, (struct symt_compiland *)compiland->compiland, &walker))) return result; compiland->are_symbols_loaded = TRUE; } @@ -4685,7 +4692,7 @@ static enum pdb_result pdb_reader_dereference_procedure(struct pdb_reader *pdb,
if (!compiland_id || compiland_id > pdb->num_compilands) return R_PDB_INVALID_ARGUMENT; compiland_id--; - stream_id = pdb->compilands[compiland_id].stream_id; + stream_id = pdb->compilands[compiland_id].compiland_stream_id;
if ((result = pdb_reader_walker_init(pdb, stream_id, &walker))) return result; walker.offset = stream_offset;
From: Eric Pouech epouech@codeweavers.com
Signed-off-by: Eric Pouech epouech@codeweavers.com --- dlls/dbghelp/pdb.c | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/dlls/dbghelp/pdb.c b/dlls/dbghelp/pdb.c index b2daa6bab0f..99204d3a744 100644 --- a/dlls/dbghelp/pdb.c +++ b/dlls/dbghelp/pdb.c @@ -4326,10 +4326,16 @@ static enum pdb_result pdb_reader_load_compiland_symbols(struct pdb_reader *pdb, break;
case S_THUNK32: + if (curr_func) WARN("not expecting thunks inside functions\n"); if ((result = pdb_reader_get_segment_address(pdb, cv_symbol->thunk_v3.segment, cv_symbol->thunk_v3.offset, &address))) goto failure; symt_new_thunk(pdb->module, compiland, cv_symbol->thunk_v3.name, cv_symbol->thunk_v3.thtype, address, cv_symbol->thunk_v3.thunk_len); + /* FIXME: we've seen S_FRAMEPROC inside S_THUNK... + * so until it's better supported, skip until end of thunk declaration + */ + walker->offset = cv_symbol->thunk_v3.pend; + if ((result = pdb_reader_symbol_skip_if(pdb, walker, S_END))) goto failure; break;
/*
From: Eric Pouech epouech@codeweavers.com
Signed-off-by: Eric Pouech epouech@codeweavers.com --- dlls/dbghelp/pdb.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/dlls/dbghelp/pdb.c b/dlls/dbghelp/pdb.c index 99204d3a744..1fbda526b62 100644 --- a/dlls/dbghelp/pdb.c +++ b/dlls/dbghelp/pdb.c @@ -4184,6 +4184,7 @@ static enum pdb_result pdb_reader_create_inline_site(struct pdb_reader *pdb, str
while (pdb_reader_read_inlinesite_annotation(pdb, annotation_walker, &opcode, &arg1, &arg2) == R_PDB_SUCCESS) { + if (opcode == BA_OP_Invalid) break; switch (opcode) { case BA_OP_CodeOffset:
From: Eric Pouech epouech@codeweavers.com
Signed-off-by: Eric Pouech epouech@codeweavers.com --- dlls/dbghelp/elf_module.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/dlls/dbghelp/elf_module.c b/dlls/dbghelp/elf_module.c index b786e4db874..af6fb916290 100644 --- a/dlls/dbghelp/elf_module.c +++ b/dlls/dbghelp/elf_module.c @@ -1247,7 +1247,7 @@ static BOOL elf_load_file_from_fmap(struct process* pcs, const WCHAR* filename, if (!modfmt) return FALSE; elf_info->module = module_new(pcs, filename, DMT_ELF, module_is_wine_host(filename, L".so"), FALSE, modbase, - fmap->u.elf.elf_size, 0, calc_crc32(fmap->u.elf.handle), + fmap->u.elf.elf_size, 0, 0, elf_get_machine(fmap->u.elf.elfhdr.e_machine)); if (!elf_info->module) {
-
Eric Pouech -
eric pouech (＠epo)