vi-eclipse / Eclipse-JDT

Umbrella repository for managing a backlog of Eclipse-JDT-related features/issues
0 stars 0 forks source link

Add caching to the "Run As..." context menu #19

Closed fedejeanne closed 10 months ago

fedejeanne commented 11 months ago

(Moved from #17)

Analyze (and, if necessary, improve) in JDT

ShahzaibIbrahim commented 10 months ago

After analyzing, Here are my answers.

  • What happens if someone uses the same (in plugin.xml) more than once?

The enablement expression will be evaluated again but the caching for individual expression is already done so the time for next enablementExpression will be significantly lower. We can also analyze how caching is working by setting the org.eclipse.core.expressions/tracePropertyResolving property to "true" in .options file in org.eclipse.core.expressions package.

  • Is it possible/meaningful to cache some evaluations?

As mentioned above, caching is already happening at lower level and it makes more sense to have caching there for each expression rather than combination of expression.

  • Is it possible to tune the call to ASTParser::createAST so that only the necessary data for org.eclipse.jdt.internal.junit.launcher.JUnit5TestFinder.isTest(ITypeBinding) is extracted?

Not sure about that.

fedejeanne commented 10 months ago

Thank you for documenting your findings!

  • Is it possible to tune the call to ASTParser::createAST so that only the necessary data for org.eclipse.jdt.internal.junit.launcher.JUnit5TestFinder.isTest(ITypeBinding) is extracted?

Not sure about that.

We should look into that too since it might speed up the whole thing. Specifically, I was thinking of maybe adding some new bits that the ASTParser can pass to the method CompilationUnitResolver::resolve (as a parameter here) so that the resolver skips unnecessary calls like CompilationUnitScope::faultInTypes (this is an assumption, I haven't checked if that call is actually needed).

Image

Could you please check that, @ShahzaibIbrahim ?

ShahzaibIbrahim commented 10 months ago

Could you please check that, @ShahzaibIbrahim ?

before I could try adding bits and control the execution of faultInTypes method, I tried shutting that of manually with if(false) and realized that Run as.. is not showing Junit Test option on sub menu, that means there is something that faultInTypes method evaluate and our jUnit5Tester class uses to evaluate the pre condition to check whether to show it or not.. In short, this needs to be executed and thus its not called unnecessarily.

fedejeanne commented 6 months ago

No meaningful optimizations/changes were introduced.