https://github.com/spring-projects/spring-framework/blob/65faca8236124e0ae37ff68afc2366f7ccf286d2/spring-r2dbc/src/main/java/org/springframework/r2dbc/core/DefaultDatabaseClient.java#L264C2-L274C4
these bind methods new too many times when many binds.
@Override
public DefaultGenericExecuteSpec bind(int index, Object value) {
assertNotPreparedOperation();
Assert.notNull(value, () -> String.format(
"Value at index %d must not be null. Use bindNull(…) instead.", index));
Map<Integer, Parameter> byIndex = new LinkedHashMap<>(this.byIndex);
byIndex.put(index, resolveParameter(value));
return new DefaultGenericExecuteSpec(byIndex, this.byName, this.sqlSupplier, this.filterFunction);
}
A batch interface can be provided or some optimize so that the following two objects can be created only once
new LinkedHashMap<>
new DefaultGenericExecuteSpec
I do a test to check the performance:
@Test
public void test() {
DefaultGenericExecuteSpec spec = new DefaultGenericExecuteSpec(() -> "select 1");
long start = System.currentTimeMillis();
for (int i = 0; i < 10000; i++){
// use the default method
spec = spec.bind(i, "string");
}
System.out.println("default method cost: " + (System.currentTimeMillis() - start));
start = System.currentTimeMillis();
for (int i = 0; i < 10000; i++){
// use the optimized method
spec = spec.bind2(i, "string");
}
System.out.println("optimized method cost: " + (System.currentTimeMillis() - start));
}
public DefaultGenericExecuteSpec bind2(int index, Object value) {
assertNotPreparedOperation();
Assert.notNull(value, () -> String.format(
"Value at index %d must not be null. Use bindNull(…) instead.", index));
if(byIndex.isEmpty()) {
Map<Integer, Parameter> byIndex = new LinkedHashMap<>(this.byIndex);
byIndex.put(index, resolveParameter(value));
return new DefaultGenericExecuteSpec(byIndex, this.byName, this.sqlSupplier, this.filterFunction);
}
byIndex.put(index, resolveParameter(value));
return this;
}
the result is
default method cost: 1790
optimized method cost: 5
more details, my project need insert large amount of data , about 3000 bind parameters per second. The cpu is high. I analyze to here according to the flame diagram.
Comment From: bclozel
Sorry but this is not enough for us to go on.
How did you measure that this was a performance bottleneck? Do you have profiling data to back this up? We would rather spend time working on performance enhancements for meaningful optimizations rather than simple hunches.
I'm closing this issue for now, we can reopen if we get actual data.
Comment From: wanyongx
Sorry but this is not enough for us to go on.
How did you measure that this was a performance bottleneck? Do you have profiling data to back this up? We would rather spend time working on performance enhancements for meaningful optimizations rather than simple hunches.
I'm closing this issue for now, we can reopen if we get actual data.
Hello @bclozel , thanks for your reply. I've added some actual data.Could you please check the issue again?
Comment From: bclozel
I have looked at this and I think you are misusing the API.
The GenericExecuteSpec
is meant to be stateless and each call should always return a new instance, because developers can reuse the spec instance for several different queries. As a result, we cannot apply the change you are suggesting because this would change the stateless nature of the spec API and this would be a regression.
If you are indeed calling bind
many times in a row, you should instead use bindValues(Map<String, ?> source)
, which should only cause a single allocation for all binding parameters. I'm closing this issue as a result.
Comment From: wanyongx
I have looked at this and I think you are misusing the API.
The
GenericExecuteSpec
is meant to be stateless and each call should always return a new instance, because developers can reuse the spec instance for several different queries. As a result, we cannot apply the change you are suggesting because this would change the stateless nature of the spec API and this would be a regression.If you are indeed calling
bind
many times in a row, you should instead usebindValues(Map<String, ?> source)
, which should only cause a single allocation for all binding parameters. I'm closing this issue as a result.
@bclozel, I know this api bindValues(Map
Comment From: bclozel
In this case, we should consider this as a variant of #27282.
There are existing equivalents with org.springframework.jdbc.core.simple.JdbcClient.StatementSpec#params(java.util.List<?>)
and org.springframework.jdbc.core.simple.JdbcClient.StatementSpec#params(java.lang.Object...)
and I think we should go with one of those.
Your bindValues(Map<String, ?> source)
suggestion would somehow infer that calling this method multiple times would only partially override the ordered parameters, which I think could be easily confusing if we mix this with org.springframework.r2dbc.core.DatabaseClient.GenericExecuteSpec#bind(int, java.lang.Object)
calls.
Comment From: bclozel
@wanyongx a new bindValues(List<?>)
variant will be available in the upcoming 6.2.0-M7, to be released in a couple of weeks.
Comment From: wanyongx
@wanyongx a new
bindValues(List<?>)
variant will be available in the upcoming 6.2.0-M7, to be released in a couple of weeks.
@bclozel That's good, bindValues(List<?>) is what I needed.I'll wait for this release.