Skip to content
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

[incubator-kie-drools-6088] Performance issue with many kbase models … #6089

Conversation

tkobayas
Copy link
Contributor

Copy link
Contributor

@pibizza pibizza left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ignore comment. Wrong PR:

@tkobayas
Copy link
Contributor Author

tkobayas commented Sep 18, 2024

Hi @mariofusco

The performance issue only arises when a KieProject has many kbase models and many large spreadsheets.

In this case, AbstractKieProject.buildKnowledgePackages is called per kbase model.

AbstractKieProject.buildKnowledgePackages calls AbstractKieProject.addFiles -> KieBuilderImpl.filterFileInKBase -> KieBuilderImpl.filterFileInKBase (called per file) -> KieBuilderImpl.isFileInKieBase -> KieBuilderImpl.packageNameForFile -> KieBuilderImpl.packageNameFromAsset -> KieBuilderImpl.packageNameFromDtable -> DecisionTableProviderImpl.loadFromResource -> SpreadsheetCompiler.compile.

If a KieProject has 20 kbase models and 20 spreadsheets, packageNameFromDtable is called 400 times. One packageNameFromDtable may not take very long time (1 or 2 seconds), but 400 times are significant.

https://github.com/apache/incubator-kie-drools/blob/main/drools-compiler/src/main/java/org/drools/compiler/kie/builder/impl/KieBuilderImpl.java#L438

This PR introduces packageNameCache and brings the cache across static methods. This doesn't look nice, but firstly I wanted to share the idea.

Another possible approach would be caching the DRL generated by SpreadsheetCompiler, but I wonder where is the appropriate place to have the cache (and when we should invalidate the cache).


Workaround for non-fixed version: Split KieContainer/ReleaseId per kbaseModel so you can avoid the combinatorial explosion.

@tkobayas
Copy link
Contributor Author

GHA ubuntu-latest: flaky

2024-09-18T09:04:13.7047738Z [ERROR] Tests run: 8, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 16.53 s <<< FAILURE! -- in org.drools.compiler.integrationtests.TimerAndCalendarFireUntilHaltTest

@mariofusco
Copy link
Contributor

Thanks a lot for your analysis of the problem @tkobayas. I implemented a very similar alternative solution, that however moves that same cache in the dtable module, trying to explain why I believe this would be a better solution. Let me know what you think about it.

@mariofusco
Copy link
Contributor

Replaced by #6097

@mariofusco mariofusco closed this Sep 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants