[GitHub] [groovy] danielsun1106 commented on issue #1135: GROOVY-8298: Slow Performance Caused by Invoke Dynamic
danielsun1106 commented on issue #1135: GROOVY-8298: Slow Performance Caused by
Invoke Dynamic
URL: https://github.com/apache/groovy/pull/1135#issuecomment-574100086
I picked the fastest result of the different version of Groovy
indy2: 4693ms; indy1: 5268ms; cs: 6007ms
**indy2 vs indy1:** 10.91% time reduced
**indy2 vs cs:** 21.87% time reduced
```groovy
final Random random = new Random()
final int limit = 1_000_000
def factorial
factorial = { n, acc = 1G -> 1 >= n ? acc : factorial(n - 1, acc * n
) }
for (int i = 0; i < limit; i++) {
factorial(30 + random.nextInt(5))
}
```
(Copied from
https://e.printstacktrace.blog/groovy-trampoline-closure-a-step-into-recursive-closures/)
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [groovy] danielsun1106 commented on issue #1135: GROOVY-8298: Slow Performance Caused by Invoke Dynamic
danielsun1106 commented on issue #1135: GROOVY-8298: Slow Performance Caused by Invoke Dynamic URL: https://github.com/apache/groovy/pull/1135#issuecomment-572156808 @blackdrag Jochen, the benchmark test code is borrowed from groovy project, I will add more test cases ;-) As for the complicated code, if you could give me some suggestion, it would be great. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [groovy] danielsun1106 commented on issue #1135: GROOVY-8298: Slow Performance Caused by Invoke Dynamic
danielsun1106 commented on issue #1135: GROOVY-8298: Slow Performance Caused by
Invoke Dynamic
URL: https://github.com/apache/groovy/pull/1135#issuecomment-571961230
@blackdrag
Jochen, I measured the performance of cs, indy1 and indy2 with JMH (
https://github.com/danielsun1106/benchmark-labs ), the result is shown as
follows.
As we can see, the performance of indy2 is improved a lot:
Benchmark |indy2 VS indy1 | indy2 VS cs
| |
CallsiteBench.dispatch_megamorphic | 3675% | 25.83%
CallsiteBench.dispatch_monomorphic | 5.72% | 188.62%
CallsiteBench.dispatch_polymorphic | 3000% | _-17.11%_
**indy2:**
```
BenchmarkMode Cnt ScoreError Units
CallsiteBench.dispatch_megamorphic thrpt 15 151.552 ± 2.157 ops/ms
CallsiteBench.dispatch_monomorphic thrpt 15 4768.491 ± 41.386 ops/ms
CallsiteBench.dispatch_polymorphic thrpt 15 155.040 ± 7.380 ops/ms
```
**indy1:**
```
BenchmarkMode Cnt ScoreError Units
CallsiteBench.dispatch_megamorphic thrpt 15 3.914 ± 0.462 ops/ms
CallsiteBench.dispatch_monomorphic thrpt 15 4510.525 ± 46.702 ops/ms
CallsiteBench.dispatch_polymorphic thrpt 15 5.283 ± 0.325 ops/ms
```
**cs:**
```
BenchmarkMode Cnt ScoreError Units
CallsiteBench.dispatch_megamorphic thrpt 15 119.447 ± 45.829 ops/ms
CallsiteBench.dispatch_monomorphic thrpt 15 1651.584 ± 39.724 ops/ms
CallsiteBench.dispatch_polymorphic thrpt 15 186.763 ± 14.549 ops/ms
```
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [groovy] danielsun1106 commented on issue #1135: GROOVY-8298: Slow Performance Caused by Invoke Dynamic
danielsun1106 commented on issue #1135: GROOVY-8298: Slow Performance Caused by
Invoke Dynamic
URL: https://github.com/apache/groovy/pull/1135#issuecomment-570983832
Here is an edge case: we set `groovy.indy.optimize.threshold` to `0`, and
the target of callsite `toString` will be set because the threshold(`0`)
reaches. The newly set target is not good for polymorfic calls(the subsequent
calls when `i >= 10`), so when fallback count reaches threshold, we will reset
to the `fromCache` target, i.e. the default target.
```groovy
for (int i = 0; i < 1_000_000; i++) {
[1, 2.0f, '3.0'].each {
if (1 != it && i < 10) return
it.toString()
}
}
```
(indy2(after d799d22, `groovy.indy.optimize.threshold`: `0`): **3s**;
indy2(before d799d22, `groovy.indy.optimize.threshold`: `0`): **12s**; indy1:
**27s**; CS: **4s** on my another machine)
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [groovy] danielsun1106 commented on issue #1135: GROOVY-8298: Slow Performance Caused by Invoke Dynamic
danielsun1106 commented on issue #1135: GROOVY-8298: Slow Performance Caused by
Invoke Dynamic
URL: https://github.com/apache/groovy/pull/1135#issuecomment-570964021
@blackdrag
Jochen, `setTarget` is executed twice when `threshold` is `0`: one for
`next`(i.e. i++) and another for `m(i)`, so it's correct ;-)
```groovy
def m(i){1}
for (int i = 0; i < 100_000_000; i++) {
if (m(i)!=1) throw new Error("??")
}
```
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [groovy] danielsun1106 commented on issue #1135: GROOVY-8298: Slow Performance Caused by Invoke Dynamic
danielsun1106 commented on issue #1135: GROOVY-8298: Slow Performance Caused by Invoke Dynamic URL: https://github.com/apache/groovy/pull/1135#issuecomment-570908778 Jochen, I'm using the JDK 11: ``` C:\Users\Daniel.LAPTOP-H7RKNSIS>java -version openjdk version "11.0.5" 2019-10-15 LTS OpenJDK Runtime Environment Corretto-11.0.5.10.1 (build 11.0.5+10-LTS) OpenJDK 64-Bit Server VM Corretto-11.0.5.10.1 (build 11.0.5+10-LTS, mixed mode) ``` >But if you check how often setTarget is called on the callsite even with a threshold of 0 you will see that there must be something wrong in the implementation. OK. I'll look into the issue later. Thanks for your reviewing again ;-) This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [groovy] danielsun1106 commented on issue #1135: GROOVY-8298: Slow Performance Caused by Invoke Dynamic
danielsun1106 commented on issue #1135: GROOVY-8298: Slow Performance Caused by
Invoke Dynamic
URL: https://github.com/apache/groovy/pull/1135#issuecomment-570900509
Jochen, as you found, the regular case the most inefficient one. So in the
latest commit I introduce a threshold to control whether to set target.
```groovy
def m(i){1}
for (int i = 0; i < 100_000_000; i++) {
if (m(i)!=1) throw new Error("??")
}
```
(indy2: before commit 6092c76: **15s**, after commit 6092c76(threshold:
100_000): **3s**, indy1: **3s**. indy2 costs almost same time to indy1 on my
machine)
Please review the PR again, thanks :-)
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [groovy] danielsun1106 commented on issue #1135: GROOVY-8298: Slow Performance Caused by Invoke Dynamic
danielsun1106 commented on issue #1135: GROOVY-8298: Slow Performance Caused by
Invoke Dynamic
URL: https://github.com/apache/groovy/pull/1135#issuecomment-570811643
Jochen, the main idea can be described as the following code:
```
if (cacheHits(receiverCassName)) { // Try to find the cached MethodHandle
// the main logic of `fromCache`
MethodHandle mh = lruCache.get(receiverCassName); // find the cached
MethodHandle again...
return mh.invokeExact(...);
} else { // fallback, and put the fallback methodhandle to the inline
cache(LRU) for reuse
// the main logic of `selectMethod`
Selector selector = Selector.getSelector(...);
selector.setCallSiteTarget();
MethodHandle mh = selector.handle.asSpreader(...).asType(...);
lruCache.put(receiverCassName, mh); // cache the methodhandle
return mh.invokeExact(...);
}
```
As the inline cache is implemented as LRU cache, and `cacheHits` and
`fromCache` are executed in two steps(not atomic), so if `cacheHits` returns
`true`(means cached methodhandle found), `fromCache` find the same cached
methodhandle from LRU cache, the result probably is `null` as cached
methodhandle may be cleared according to the rule of LRU... I use `ThreadLocal`
to avoid the above concurrent issue.
Also, we have inline cache for callsite, so `setTarget` can be avoided,
which causes the poor performance as the GROOVY-8298 shown.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [groovy] danielsun1106 commented on issue #1135: GROOVY-8298: Slow Performance Caused by Invoke Dynamic
danielsun1106 commented on issue #1135: GROOVY-8298: Slow Performance Caused by Invoke Dynamic URL: https://github.com/apache/groovy/pull/1135#issuecomment-570689193 @blackdrag Jochen, it would be great if you could review the PR :-) This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
