We concluded that sample-below 4099 beats sample-below 4096. I presume 4093 would perform about the same as 4099, except we'll record 4096-byte allocs precisely, although we'd of course want to test that. Anyway, whatever default we think is best, we should expose that as a default. People shouldn't have to remember --sample-below=4093. I propose we call this arg "--sample", or maybe "--sample-default".
Actually, I think we should make sampling the default. Let's make this as easy as possible.
Summary: Add arg to DMD which sets us up to sample-below ~ 4096 → Default DMD to sample size ~4096
Created attachment 690942 [details] [diff] [review] Patch, v1 Also print out the sample size, since it's no longer explicit in the $DMD envvar.
Attachment #690942 - Flags: review?(n.nethercote)
Comment on attachment 690942 [details] [diff] [review] Patch, v1 Review of attachment 690942 [details] [diff] [review]: ----------------------------------------------------------------- Heh, I was just thinking about asking you if you thought this was a good idea :) At the top of RunTestMode(), just after the |Writer| declaration, please set |gSampleBelowSize| to 1 and add a comment "The first part of the test requires sampling to be off." In general, if you're going to be modifying the output, you'll need to be testing with --mode=test. That might require me making it more robust... it currently fails for me on Mac because a couple of unexpected system lib mallocs occur. I filed bug 820566 for this. ::: memory/replace/dmd/DMD.cpp @@ +1754,5 @@ > size_t totalUsableSize = unreportedUsableSize + reportedUsableSize; > > WriteTitle("Invocation\n"); > + W("$DMD = '%s'\n", gDMDEnvVar); > + W("Sample-below size = %lld\n\n", (long long)(gSampleBelowSize)); You'll need to update memory/replace/dmd/test-expected.dmd to include this line, once for each of the four sections.
Attachment #690942 - Flags: review?(n.nethercote) → review+
Created attachment 691105 [details] test.dmd generated with this pach on B2G Doesn't look quite right, but the part affected by this patch is fine. :)
Assignee: nobody → justin.lebar+bug
Summary: Default DMD to sample size ~4096 → Default DMD to sample-below=4093
> Doesn't look quite right, but the part affected by this patch is fine. :) Um...
(In reply to Nicholas Nethercote [:njn] from comment #6) > Um... I'm not quite sure what that means. :) I can't say I'm terribly concerned that the test doesn't work right on B2G given that DMD itself seems to work fine.
It's just weird because the test is doing pretty basic stuff, and the output is not even remotely right -- it's like it's failing to intercept every call to malloc. I wonder if the fact that the malloc calls are coming from within the same file causes them to be missed on B2G.
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla20
Comment on attachment 690942 [details] [diff] [review] Patch, v1 [Triage Comment] DMD is npotb.
Attachment #690942 - Flags: approval-mozilla-beta+ → approval-mozilla-b2g18+
status-b2g18: --- → fixed
status-firefox19: --- → fixed
status-firefox20: --- → fixed
You need to log in before you can comment on or make changes to this bug.