`DefaultRedisScript` checks for `isModified` in a `synchronized` block that affects multi-threaded performance
org.springframework.data.redis.core.script.DefaultRedisScript
synchronized seriously affects the performance of executing redis lua scripts
Usually, the redis lua script will not change
Please refrain from pasting images into bug reports as these cannot be searched or indexed. Instead, use stable links to code such as the following one:
https://github.com/spring-projects/spring-data-redis/blob/b57a1e0376a55c42eb3f8be5cc1a6ebfe7b33afc/src/main/java/org/springframework/data/redis/core/script/DefaultRedisScript.java#L85-L93
The code could be improved by limiting the synchronized block to the sha1 assignment. In return, the sha1 field can become subject to visibility issues across multiple threads which can lead to multiple computations of the SHA value but that is the flip side of best effort caching.
Feel free to submit a pull request.
@mp911de can I contribute in this to kick start my journey ? if no one is working on this.
Sure, feel free.
public String getSha1() {
if (sha1 == null || scriptSource.isModified()) {
synchronized (shaModifiedMonitor) {
this.sha1 = DigestUtils.sha1DigestAsHex(getScriptAsString());
}
}
return sha1;
}
private @Nullable ScriptSource scriptSource; // line no. 42
@mp911de method invocation scriptSource.isModified() may produce NullPointerException. we need to handle this too, please correct me if wrong here..
DefaultRedisScript is a bean that incorporates lifecycle via InitializingBean. I took a look at the problem and introduced a general revision of all synchronized blocks that lead to blocking calls via #2691