[PATCH] D16726: [Profiling] Speed up unittests by ~5x

[David Blaikie via llvm-commits]

On Fri, Jan 29, 2016 at 9:15 AM, Xinliang David Li <davidxl at google.com>
wrote:

> 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...


>
> David
>
> 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,
> for
> > 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...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160129/d43bc42f/attachment.html>