Closed Bug 475165 Opened 16 years ago Closed 14 years ago

Improve compression used on pdb files in symbol store


(Toolkit :: Crash Reporting, defect)

Windows XP
Not set





(Reporter: RyanVM, Assigned: RyanVM)




(1 file, 2 obsolete files)

As mentioned in bug 385792 comment #14 and on, makecab supports higher compression settings than default. The attached one-liner adds those settings. Using those settings on the xul.pdb Ted created for bug 385792, the resulting xul.pd_ file comes out to 12.3MB instead of 18.4MB with the default settings.

I haven't tested this patch or the resulting pdb file since I don't build crashreporter for my own builds, so someone will want to test it before checking in. However, being that they've been part of the CAB format for over 10 years, I don't foresee any problems in using them.

I don't believe that removing the .exe from the makecab call will be problematic, but if it does, feel free to add it back. I took it off to save a little space.
Attachment #358608 - Flags: review?(ted.mielczarek)
Comment on attachment 358608 [details] [diff] [review]
Use increased compression settings when calling makecab

-        success = call(["makecab.exe", full_path, compressed_file],
+        success = call(["makecab /D CompressionType=LZX /D CompressionMemory=21", full_path, compressed_file],
                        stdout=open("NUL:","w"), stderr=STDOUT)

Not quite right, note that each entry of that list is a parameter to the commandline. You want something like:
["makecab.exe", "/D", ... ]
Attachment #358608 - Flags: review?(ted.mielczarek) → review-
BTW, LZX was added to the cab format when Jonathan Forbes went to work for MS back in 1997.
Let's see if this is more to your liking. As was the case before, this is completely untested by me, so I recommend you try it out before committing :-)
Attachment #358608 - Attachment is obsolete: true
Attachment #360182 - Flags: review?(ted.mielczarek)
Comment on attachment 360182 [details] [diff] [review]
Use increased compression settings when calling makecab, try 2

Looks good, I'll try to find some time to give it a go before I push it. :)
Attachment #360182 - Flags: review?(ted.mielczarek) → review+
Find any time to give it a go yet? 33% wins are nice :)
No, and I'm unlikely to. If you want this, you'll have to test it yourself or get someone else to.
K Ted, it's been a year now. I don't have the ability to test this. If it's ever going to get in, I'm going to need some help. If you can't test it, can you recommend someone else who can?
Attached patch Updated to tipSplinter Review
Carrying over ted's previous r+. I've tested this patch now using make buildsymbols. PDB compression goes fine.
Without: 43868KB
With: 33678KB

A 23% reduction in size!
Attachment #360182 - Attachment is obsolete: true
Attachment #456351 - Flags: review+
Keywords: checkin-needed
Closed: 14 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b2
Ted, do we want this on 1.9.1 and 1.9.2 as well? Should be safe enough.
Feel free to nominate and convince drivers. The symbol server is widely used by both developers and testers.
You need to log in before you can comment on or make changes to this bug.