[PATCH] D16726: [Profiling] Speed up unittests by ~5x
David Blaikie via llvm-commits
llvm-commits at lists.llvm.org
Fri Jan 29 09:24:34 PST 2016
On Fri, Jan 29, 2016 at 9:15 AM, Xinliang David Li <davidxl at google.com>
> The iteration is also encoded in the name.
That it tests compression? If we're using a standard compression algorithm,
I would expect we would just test the compression algorithm directly. I
don't think there's any need to test that the compression algorithm works
in any particular use case (test that the contents is compressed).
The input seems pretty arbitrarily chosen - it's not clear why that
provides the right tradeoff between time and coverage, it looks a bit
scattershot ("if we put enough stuff through this, perhaps it'll give
confidence that it works") - maybe something better suited to ASan Fuzzing
rather than a unit test.
Blanket testing with broad inputs probably isn't suitable for a unit test.
A single test that demonstrates that the contents is smaller compressed (if
there's a non-compressed option) might be worthwhile. Though it looks like
this test doesn't even test that - I assume I could remove the compression
support from this library & still pass the test...
> On Fri, Jan 29, 2016 at 9:03 AM, David Blaikie <dblaikie at gmail.com> wrote:
> > If we're asking the question - why does this need to be 1024? What's the
> > specific purpose of the iteration? (why would 1 be too few iterations,
> > example?)
> > On Fri, Jan 29, 2016 at 9:01 AM, David Li via llvm-commits
> > <llvm-commits at lists.llvm.org> wrote:
> >> davidxl accepted this revision.
> >> davidxl added a comment.
> >> This revision is now accepted and ready to land.
> >> lgtm
> >> http://reviews.llvm.org/D16726
> >> _______________________________________________
> >> llvm-commits mailing list
> >> llvm-commits at lists.llvm.org
> >> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the llvm-commits