You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jmeter.apache.org by "vlsi (via GitHub)" <gi...@apache.org> on 2023/05/07 12:03:49 UTC

[GitHub] [jmeter] vlsi opened a new pull request, #5885: feat: use ServiceLoader to find implementations instead of searching classes in jars

vlsi opened a new pull request, #5885:
URL: https://github.com/apache/jmeter/pull/5885

   ## Description
   
   ServiceLoader is Java standard approach for locating implementaitons, and it allows pluggability without relying on a filesystem layout.
   
   The change is backward-compatible, so JMeter searches implementations via `ServiceLoader`, and then it scans the jars like it was doing previously. However, all the JMeter-provided implementations would be provided as services.
   
   Fixes https://github.com/apache/jmeter/issues/5883
   
   TODO:
   * [x] migrate `JMeterUtils#findClassesThatExtend` usages to `ServiceLoader`
   * [ ] migrate `ClassFinder#findClassesThatExtend` usages to `ServiceLoader`
   * [ ] Add Jar manifest entry `JMeter-skip-scan-clases: true` so `ClassFinder` ignores JMeter jars
   * [ ] Add javadocs when appropriate
   * [ ] Add changelog entry
   
   ## Motivation and Context
   
   The current `JMeterUtils#findClassesThatExtend` and `ClassFinder.findClassesThatExtend` impose issues:
   1) They expect classes to be packaged in jars. It is inconvenient for testing purposes where the dependencies might come form `classes` folders
   2) Class scanning takes time. The current workaround is to use `String contains, String notContains` parameters, however, it does not resolve the issue, and it makes an awkward API overall


-- 
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: dev-unsubscribe@jmeter.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [jmeter] vlsi commented on pull request #5885: feat: use ServiceLoader to find implementations instead of searching classes in jars

Posted by "vlsi (via GitHub)" <gi...@apache.org>.
vlsi commented on PR #5885:
URL: https://github.com/apache/jmeter/pull/5885#issuecomment-1542618344

   I am afraid we have too many changes anyway on the main branch.
   
   At the same time, I think Xalan is not the only security fix we need to implement.
   
   Do you suggest creating a bugfix branch with xalan only changes?
   
   I suggest polishing what we have and releasing it as 5.6.
   I do not expect it to have major breakages or major breakthroughs, however, we have updated many dependencies, and we fixed several annoying bugs, so 5.6 sounds a good version for me.


-- 
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: dev-unsubscribe@jmeter.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [jmeter] vlsi commented on pull request #5885: feat: use ServiceLoader to find implementations instead of searching classes in jars

Posted by "vlsi (via GitHub)" <gi...@apache.org>.
vlsi commented on PR #5885:
URL: https://github.com/apache/jmeter/pull/5885#issuecomment-1542605418

   I think the PR is good to go (if the tests pass). I will merge it soon provided there are no objections.
   


-- 
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: dev-unsubscribe@jmeter.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [jmeter] vlsi commented on pull request #5885: feat: use ServiceLoader to find implementations instead of searching classes in jars

Posted by "vlsi (via GitHub)" <gi...@apache.org>.
vlsi commented on PR #5885:
URL: https://github.com/apache/jmeter/pull/5885#issuecomment-1542591751

   It looks like the extra overhead for annotation processing is negligible, so I think we can live with it.
   
   I've tried `./gradlew testClasses --rerun-tasks --no-build-cache`, and it looks like the difference does not exceed 7 seconds.
   
   #### with annotation processing
   
   ```
   BUILD SUCCESSFUL in 1m 36s
   197 actionable tasks: 197 executed
   ```
   
   https://ge.apache.org/s/td4x7pa3gnddm
   
   #### before this PR
   
   ```
   BUILD SUCCESSFUL in 1m 29s
   186 actionable tasks: 186 executed
   ```
   
   https://ge.apache.org/s/wevuaxhuqoonw
   


-- 
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: dev-unsubscribe@jmeter.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [jmeter] vlsi merged pull request #5885: feat: use ServiceLoader to find implementations instead of searching classes in jars

Posted by "vlsi (via GitHub)" <gi...@apache.org>.
vlsi merged PR #5885:
URL: https://github.com/apache/jmeter/pull/5885


-- 
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: dev-unsubscribe@jmeter.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [jmeter] vlsi commented on pull request #5885: feat: use ServiceLoader to find implementations instead of searching classes in jars

Posted by "vlsi (via GitHub)" <gi...@apache.org>.
vlsi commented on PR #5885:
URL: https://github.com/apache/jmeter/pull/5885#issuecomment-1537450211

   @FSchumacher , @emilianbold, do you remember the reasons for https://github.com/apache/jmeter/commit/574ceb61aed9de91147aa72b71a6097e6258eef3 by chance?


-- 
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: dev-unsubscribe@jmeter.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [jmeter] milamberspace commented on pull request #5885: feat: use ServiceLoader to find implementations instead of searching classes in jars

Posted by "milamberspace (via GitHub)" <gi...@apache.org>.
milamberspace commented on PR #5885:
URL: https://github.com/apache/jmeter/pull/5885#issuecomment-1542614018

   @vlsi you want release 5.5.1 with xalan security fix before merge this PR? Or we prepare the next release 5.6 or 6.0?


-- 
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: dev-unsubscribe@jmeter.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org