Closed Bug 1451008 Opened 4 years ago Closed 4 years ago

Disable gtest benchmarking on asan builds

Categories

(Testing :: GTest, enhancement)

Version 3
enhancement
Not set
normal

Tracking

(firefox61 fixed)

RESOLVED FIXED
mozilla61
Tracking Status
firefox61 --- fixed

People

(Reporter: wlach, Assigned: wlach)

Details

(Whiteboard: [PI:April])

Attachments

(1 file)

Something i noticed when reviewing bug 1437895. We currently run the benchmarks on asan builds, but I don't think this is terribly useful: pgo and opt builds are more representative of performance in the field.
Whiteboard: [PI:April]
Nathan, I flagged you review on this because you were the last person to touch this file. If anyone has objections they can make them and be heard. :)
Comment on attachment 8964588 [details]
Bug 1451008 - Disable perfherder reporting for microbenchmarks on asan builds

https://reviewboard.mozilla.org/r/233306/#review238868

On the one hand, I think it's pretty dumb to record benchmark numbers for our ASan builds; we don't really care about the performance of such builds.  On the other hand, it's not a bad idea to run as much code as we can under ASan, so we can make sure we're not doing anything dumb in said code, which could invalidate the benchmark numbers we are actually recording.  Is it not possible to change the recording in a mozharness file somewhere?

r=me, but I would like to see if there's an alternative route to victory here.

::: testing/gtest/mozilla/MozGTestBench.cpp:21
(Diff revision 1)
>  
>  namespace mozilla {
>  void GTestBench(const char* aSuite, const char* aName,
>                  const std::function<void()>& aTest)
>  {
> -#ifdef DEBUG
> +#if defined(DEBUG) || defined(MOZ_ASAN)

Hm....(see comment above)
Attachment #8964588 - Flags: review?(nfroyd) → review+
(In reply to Nathan Froyd [:froydnj] from comment #3)
> Comment on attachment 8964588 [details]
> Bug 1451008 - Disable perfherder reporting for microbenchmarks on asan builds
> 
> https://reviewboard.mozilla.org/r/233306/#review238868
> 
> On the one hand, I think it's pretty dumb to record benchmark numbers for
> our ASan builds; we don't really care about the performance of such builds. 
> On the other hand, it's not a bad idea to run as much code as we can under
> ASan, so we can make sure we're not doing anything dumb in said code, which
> could invalidate the benchmark numbers we are actually recording.  Is it not
> possible to change the recording in a mozharness file somewhere?

In fact we still do run the tests under asan (and debug), although only once as opposed to multiple times. 

I could change it to log to a different framework (not present in our perfherder config so won't appear in treeherder/perfherder), but otherwise keep the behavior exactly the same.
(In reply to William Lachance (:wlach) (use needinfo!) from comment #4)
> In fact we still do run the tests under asan (and debug), although only once
> as opposed to multiple times. 

Oh, you're right!  I am stupid and can't read a patch properly.  The current patch is OK, carry on!
Pushed by wlachance@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/da0a77f37d80
Disable perfherder reporting for microbenchmarks on asan builds r=froydnj
https://hg.mozilla.org/mozilla-central/rev/da0a77f37d80
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
You need to log in before you can comment on or make changes to this bug.