nikhiln64 commented on PR #6809: URL: https://github.com/apache/incubator-kie-drools/pull/6809#issuecomment-4994476937
Thanks for the careful look @mariofusco - I dug into the history before this PR and again now, and I'd like to share what I found, because I think this PR completes an old fix rather than re-attempts a reverted one. **The past attempt appears to have stuck, not reverted:** parallel-capable registration was introduced by 4381219ecf / #1874 (RHPAM-795/RHPAM-55, 2018) - and notably it was itself a **deadlock fix** (the commit adds "a test for a classloader deadlock"). That registration is still present today in `ProjectClassLoader` (drools-wiring-api, line 55), together with the descendant of that deadlock regression test (`ClassLoaderTest` in drools-wiring-dynamic). Searching `-S registerAsParallelCapable` across all branches I find no revert - only the 2018 introduction and the 2019 wiring-module split (#2652). If the revert you remember was in a different repo (jBPM/RHPAM product layer?), I'd genuinely appreciate the pointer. **The gap this PR closes:** when the split created the concrete subclasses, the per-class registration wasn't carried over - and the JVM checks the loader's *runtime* class, so the 2018 registration is silently inert for every `DynamicProjectClassLoader`/`StaticProjectClassLoader` instance. `getClassLoadingLock` returns `this`, i.e. exactly the coarse-lock configuration the 2018 fix was eliminating. **Relation to #6758:** the reporter runs unit tests where each scenario loads a test-specific subset of rules concurrently and hits the single-monitor serialization. So the benefit claimed here is not raw throughput - it's (a) restoring the deadlock-avoidance property of #1874 for the loaders actually instantiated at runtime, and (b) unblocking concurrent class loading for use cases like the reporter's. That said, you have institutional memory I don't. I'm happy to (1) add a JMH or multi-threaded stress benchmark quantifying the concurrent-load behavior if that would help, or (2) park this if the team's risk assessment says the coarse lock is the safer default - your call either way. -- 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. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
