-
Notifications
You must be signed in to change notification settings - Fork 86
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Performance issue due to repeated attempts to find classes without FQN #306
Comments
Thanks for the PR. Of course, I will look at your contribution benevolently. To convince me that this actually solves a real-world problem and not just take your word for it, even though what you say sounds plausible, please provide a reproducer for the issue, i.e. some kind of sample program and configuration that makes AspectJ slow before and significantly faster after your proposed change. |
Hi @kriegaex - thanks for the quick response. In this case the first three jars are required for the classpath the rest are unnecessary but fulfill the example - the more jars you add the slower you will see the setup to be, the same is true with more aspects that have similar characteristics. With ~1000 jars on the classpath the slowdown is notable EDIT: added commons-logging to the classpath for completeness. |
Thank you. Please also attach the source code and ideally also the Maven coordinates for the classpath libraries. |
The project to generate the sample jar is below. The maven coordinates for required classpath libraries are in the pom for that, other items on the classpath are there to prove the problem and can largely be anything you choose. There may be some other improvement mechanism possible that means these incomplete class names are not searched for in the first place. In this example I believe that they originate from the pointcut syntax which contains simple names from either the local namespace or |
With this project and the handful of dependencies you suggested I cannot detect any noticeable difference in performance. I need the suggested change to be verifiable and measurable. Are you expecting me to create a project with 1,000 libraries from scratch? Like I said, your PR is welcome, but you need to sell it a bit better to convince me. |
I think I found a simple way to unit-test the performance. The disadvantage of my test approach is a fixed timeout, which might cause tests to be brittle, but unless I keep the old way to search find types and add a toggle to be able to compare both approaches, there is no easy way to verify the PR's effectiveness. |
@kriegaex I think that my original profiling may have highlighted this area more due to my usage tipping over the 1000 threshold that is the default for To see an impact I upped the number of aspects to 10 (by copying) and generated ~1000 further jars for the classpath (copying using a script to get e.g. anyjar00001.jar, ..., anyjar.00995.jar - this kept it below 1000 in total so not triggering the file closing) |
Performance regression test for eclipse-aspectj#306, failing before the enhancement and passing afterward. Signed-off-by: Alexander Kriegisch <Alexander@Kriegisch.name>
Fixes eclipse-aspectj#306 and adds a few small optimisations. Co-authored-by: Alexander Kriegisch <Alexander@Kriegisch.name> Signed-off-by: Alexander Kriegisch <Alexander@Kriegisch.name>
Performance regression test for #306, failing before the enhancement and passing afterward. Signed-off-by: Alexander Kriegisch <Alexander@Kriegisch.name>
Fixes #306 and adds a few small optimisations. Co-authored-by: Alexander Kriegisch <Alexander@Kriegisch.name> Signed-off-by: Alexander Kriegisch <Alexander@Kriegisch.name>
Merged including my test. Thank you very much for your contribution, @harrisric. |
Hi, I've been using aspectj with a very large classpath and seeing some performance issues recently. Profiling showed
ClassPathManager.find
as a hotspot.Further investigation led me to see that we end up repeatedly looking for classes based on an incomplete class name and that this was slow because each jar on the classpath was rechecked each time the class is referenced.
The examples of this that I found were where the aspect would have a simple class name without an import, typically things from the
java.lang
namespace such asString
orObject
, or else classes that are in the same package as the aspect.Implementing a simple cache of class names that weren't found generated a significant speed up.
The text was updated successfully, but these errors were encountered: