frankban / quicktest

Quick helpers for testing Go applications
MIT License
529 stars 27 forks source link

Faster C.Run by using less reflect #160

Closed dolmen closed 1 year ago

dolmen commented 1 year ago

In C.Run bypass check of the signature of the Run method of c.TB by using a prefilled table for the common cases of *testing.T, *testing.B and *quicktest.C.

This serie of patches shows the developement process:

$ go test -bench BenchmarkCRunGetFuncSig -benchmem
    BenchmarkCRunGetFuncSig/*testing.T-no-cache-4          562579   2142     ns/op   168 B/op             4 allocs/op
    BenchmarkCRunGetFuncSig/*testing.T-with-cache-4     183638598      7.514 ns/op     0 B/op             0 allocs/op
    BenchmarkCRunGetFuncSig/*testing.B-no-cache-4          608715   2626     ns/op   168 B/op             4 allocs/op
    BenchmarkCRunGetFuncSig/*testing.B-with-cache-4     100000000     11.00  ns/op     0 B/op             0 allocs/op
    BenchmarkCRunGetFuncSig/*quicktest.C-no-cache-4        636226   2523     ns/op   168 B/op             4 allocs/op
    BenchmarkCRunGetFuncSig/*quicktest.C-with-cache-4    68991796     48.91  ns/op     0 B/op             0 allocs/op
dolmen commented 1 year ago

@frankban Please merge #157 first. I'll do the rebase of #160 on top.

dolmen commented 1 year ago

@frankban Rebased.

This important optimization (with my other fixes) are worth a release.

dolmen commented 1 year ago

ping @frankban

frankban commented 1 year ago

This looks very interesting. We are discussing with @rogpeppe about other possible ways for introducing this optimization. Thanks for this work!

dolmen commented 1 year ago

I could go further and completely remove the getRunFuncSignature. At this point the cache is statically filled and getRunFuncSignature is only used at runtime for error reporting in the case of a bad usage (and also for the benchmark that would go too): this could be replaced by a simple message displaying the signature of the received function and the list of acceptable ones.

An alternate implementation would use generics to enforce the signature at compile time. But are you ready to push the minimum Go version required from 1.13 to 1.18?

dolmen commented 1 year ago

Well, Go doesn't have generic methods yet, so I don't see how a compile-time check of the subtest signature could be enforced.

dolmen commented 1 year ago

Rebased on top of v1.14.6.

dolmen commented 1 year ago

Replaced by #165 which completely bypass reflect in the common cases for which we used a cache here.