Support forkjoinpool plugin in JDK11 #656
Conversation
|
You need to add the test scenario to run in JDK 11, otherwise, this is only a manual test |
|
Also, you removed the pull request template, we need that, and you missed the update in the change log. |
This is still missed. |
|
Why do you open a new test3 with so many cases? |
| strategy: | ||
| matrix: | ||
| case: | ||
| - jdk11-forkjoinpool-scenario |
There was a problem hiding this comment.
Do we need a new case? I think this is rt.jar(jdk level) change, even the codes are compiled as 1.8, the runtime method is still going to be forced to run in 11, right?
There was a problem hiding this comment.
Could we not copy the test scenario, but directly write the existing test in jdk11. Was that failing before and running now?
There was a problem hiding this comment.
yes, you are right. I will optimize it
| @@ -33,6 +33,9 @@ public class ForkJoinWorkerQueueInstrumentation extends ClassInstanceMethodsEnha | |||
| private static final String FORK_JOIN_WORKER_QUEUE_CLASS = "java.util.concurrent.ForkJoinPool$WorkQueue"; | |||
|
|
|||
| private static final String FORK_JOIN_WORKER_QUEUE_RUN_TASK_METHOD = "runTask"; | |||
|
|
|||
| private static final String FORK_JOIN_WORKER_QUEUE_RUN_TASK_METHOD_JDK11 = "topLevelExec"; | |||
There was a problem hiding this comment.
Please add comments about why there are two methods to intercept.
|
From test logs, your interceptor doesn't run as expected. Still, the old method existed and being intercepted. |
|
Tests passed, could you check this? #656 (comment) |
|
Is there any update here? |
|
@786991884 Do you have any update? |
|
@786991884 If you will be back, I hope this extra test can be removed by reusing the existing one. I am going to merge this first because from what I saw, this fix is important. |
fix apache/skywalking#11633