Support non-uniform batch entries #2110

pingw33n · 2022-09-22T14:59:51Z

Closes #2114 #2110 #2078 #1968

pingw33n · 2022-09-23T08:31:57Z

@hgschmie @stevenschlansker Seems like CI failed due unrelated reason.

As all of the mappers deal with mapping data onto beans (which they use the setters for), they should also prefer the annotations found on the setter over the annotations on the getter. This PR also splits the very unwieldy and confusing BeanMapperTest into three separate tests, one focusing on general bean mapper properties, one for the PropagateNullable tests and one for Nested tests.

LGTM is dead, long live sonarcloud

pingw33n · 2022-09-30T10:23:48Z

@hgschmie Updated this PR to be a general fix for the issues.

Benchmarks (done on MacBook M1 Pro, 16G RAM):

master:

Benchmark                                  Mode  Cnt   Score   Error  Units
BeanBindingBenchmark.batchJdbc            thrpt    5  12.507 ± 0.707  ops/s
BeanBindingBenchmark.batchJdbiBean        thrpt    5  12.614 ± 0.782  ops/s
BeanBindingBenchmark.batchJdbiMap         thrpt    5   9.499 ± 0.492  ops/s
BeanBindingBenchmark.batchJdbiNamed       thrpt    5   9.478 ± 1.121  ops/s
BeanBindingBenchmark.batchJdbiPositional  thrpt    5   8.074 ± 0.518  ops/s
BeanBindingBenchmark.oneJdbc              thrpt    5  37.886 ± 1.665  ops/s
BeanBindingBenchmark.oneJdbi              thrpt    5  22.739 ± 0.190  ops/s

This PR:

Benchmark                                  Mode  Cnt   Score   Error  Units
BeanBindingBenchmark.batchJdbc            thrpt    5  12.592 ± 0.488  ops/s
BeanBindingBenchmark.batchJdbiBean        thrpt    5   9.311 ± 0.213  ops/s
BeanBindingBenchmark.batchJdbiMap         thrpt    5   7.692 ± 0.246  ops/s
BeanBindingBenchmark.batchJdbiNamed       thrpt    5   8.748 ± 0.348  ops/s
BeanBindingBenchmark.batchJdbiPositional  thrpt    5   9.779 ± 0.671  ops/s
BeanBindingBenchmark.oneJdbc              thrpt    5  42.860 ± 2.479  ops/s
BeanBindingBenchmark.oneJdbi              thrpt    5  21.851 ± 0.390  ops/s

This PR without support for non-uniform batch entries (just the refactoring):

Benchmark                                  Mode  Cnt   Score   Error  Units
BeanBindingBenchmark.batchJdbc            thrpt    5  11.418 ± 0.518  ops/s
BeanBindingBenchmark.batchJdbiBean        thrpt    5  12.099 ± 0.617  ops/s
BeanBindingBenchmark.batchJdbiMap         thrpt    5   9.369 ± 0.422  ops/s
BeanBindingBenchmark.batchJdbiNamed       thrpt    5   9.987 ± 0.871  ops/s
BeanBindingBenchmark.batchJdbiPositional  thrpt    5   8.920 ± 0.442  ops/s
BeanBindingBenchmark.oneJdbc              thrpt    5  37.719 ± 0.382  ops/s
BeanBindingBenchmark.oneJdbi              thrpt    5  22.761 ± 0.361  ops/s

This PR, Postgres 14.5

Benchmark                                  Mode  Cnt  Score   Error  Units
BeanBindingBenchmark.batchJdbc            thrpt    5  1.724 ± 0.378  ops/s
BeanBindingBenchmark.batchJdbiBean        thrpt    5  1.869 ± 0.070  ops/s
BeanBindingBenchmark.batchJdbiMap         thrpt    5  1.754 ± 0.083  ops/s
BeanBindingBenchmark.batchJdbiNamed       thrpt    5  1.859 ± 0.071  ops/s
BeanBindingBenchmark.batchJdbiPositional  thrpt    5   8.400 ± 0.685  ops/s
BeanBindingBenchmark.oneJdbc              thrpt    5  0.160 ± 0.016  ops/s
BeanBindingBenchmark.oneJdbi              thrpt    5  0.143 ± 0.038  ops/s

This PR, Postgres 14.5, 200000 batch

Benchmark                                  Mode  Cnt  Score   Error  Units
BeanBindingBenchmark.batchJdbc            thrpt    5  0.430 ± 0.276  ops/s
BeanBindingBenchmark.batchJdbiBean        thrpt    5  0.436 ± 0.063  ops/s
BeanBindingBenchmark.batchJdbiMap         thrpt    5  0.385 ± 0.049  ops/s
BeanBindingBenchmark.batchJdbiNamed       thrpt    5  0.423 ± 0.038  ops/s
BeanBindingBenchmark.batchJdbiPositional  thrpt    5  0.413 ± 0.042  ops/s
BeanBindingBenchmark.oneJdbc              thrpt    5  0.163 ± 0.007  ops/s
BeanBindingBenchmark.oneJdbi              thrpt    5  0.160 ± 0.004  ops/s

My comments/thoughts:

This PR makes batchJdbiBean slower 35%, batchJdbiNamed slower 8%.
As seen in the benchmarks against Postgres, in real world setup it's unlikely the bottleneck would be in JDBI. And where it would matter (e.g. in setup with distributed DB) JDBI probably won't fit at all. Also it would be possible to expose configuration option to enable strictly-typed batches.
It works by computing cache key for each batch entry from arguments in Bindings. The caching will still break if different NamedArgumentFinder sets are used in different batch entries. Could be improved via getNames().
Even without the fix I think the refactoring part would be useful improvement to the codebase since it removes code duplication and makes it more readable.
This PR still needs clean up.