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

Cache compiled dtables during the building of a KieProject #6097

Merged
merged 3 commits into from
Sep 24, 2024

Conversation

mariofusco
Copy link
Contributor

Fixes #6088 and provide an alternative solution to what implemented in #6089.

I believe this is a better solution for a twofold reason:

  1. The cache is confined in the dtable module and only exists when dtables are actually used.
  2. With the other solution the xls -> drl conversion was still performed twice, once for the actual compilation and the other to retrieve the package name, while moving the cache to a lower level allows to effectively compile a dtable only once.

}

private static void clearBuilderCache() {
DecisionTableFactory.clearCompilerCache();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

In general I believe that there could be other things that we may want to cache when building a KieProject, so we could eventually perform the clean up of all those caches here.

@kie-ci3
Copy link

kie-ci3 commented Sep 23, 2024

PR job #1 was: UNSTABLE
Possible explanation: This should be test failures

Reproducer

build-chain build full_downstream -f 'https://raw.githubusercontent.com/${AUTHOR:apache}/incubator-kie-kogito-pipelines/${BRANCH:main}/.ci/buildchain-config-pr-cdb.yaml' -o 'bc' -p apache/incubator-kie-drools -u #6097 --skipParallelCheckout

NOTE: To install the build-chain tool, please refer to https://github.com/kiegroup/github-action-build-chain#local-execution

Please look here: https://ci-builds.apache.org/job/KIE/job/drools/job/main/job/pullrequest_jobs/job/drools-pr/job/PR-6097/1/display/redirect

Test results:

  • PASSED: 20109
  • FAILED: 10

Those are the test failures:

org.drools.decisiontable.SpreadsheetIntegrationExampleTest.testHeadingWhitespace
Expecting actual not to be null
org.drools.decisiontable.SpreadsheetIntegrationExampleTest.testNamedWorksheet
Expected size: 1 but was: 0 in:
[]
org.drools.decisiontable.project.MultiSheetsTest.testSheet12 Unable to create KieModule, Errors Existed: [Message [id=1, kieBase=dtblaleKB, level=ERROR, path=org/drools/simple/candrink/CanDrink.drl.xls, line=5, column=0
text=Duplicate rule name: ID change_11], Message [id=2, kieBase=dtblaleKB, level=ERROR, path=org/drools/simple/candrink/CanDrink.drl.xls, line=13, column=0
text=Duplicate rule name: ID change_12], Message [id=3, kieBase=dtblaleKB, level=ERROR, path=org/drools/simple/candrink/CanDrink.drl.xls, line=13, column=0
text=Rule Compilation error The type Rule_ID_change_12178611955 is already defined], Message [id=4, kieBase=dtblaleKB, level=ERROR, path=org/drools/simple/candrink/CanDrink.drl.xls, line=5, column=0
text=Rule Compilation error The type Rule_ID_change_111481680058 is already defined]]
org.drools.decisiontable.project.MultiSheetsTest.testSheet2
Expecting actual:
"[Mario can drink]"
to contain:
"Mario can drive"
org.drools.compiler.integrationtests.MultiSheetsTest.testSheet12[KieBase type=CLOUD_IDENTITY] Unable to create KieModule, Errors Existed: [Message [id=1, kieBase=dtblaleKB, level=ERROR, path=org/drools/simple/candrink/CanDrink.drl.xls, line=5, column=0
text=Duplicate rule name: ID change_11], Message [id=2, kieBase=dtblaleKB, level=ERROR, path=org/drools/simple/candrink/CanDrink.drl.xls, line=13, column=0
text=Duplicate rule name: ID change_12], Message [id=3, kieBase=dtblaleKB, level=ERROR, path=org/drools/simple/candrink/CanDrink.drl.xls, line=13, column=0
text=Rule Compilation error The type Rule_ID_change_12178611955 is already defined], Message [id=4, kieBase=dtblaleKB, level=ERROR, path=org/drools/simple/candrink/CanDrink.drl.xls, line=5, column=0
text=Rule Compilation error The type Rule_ID_change_111481680058 is already defined]]
org.drools.compiler.integrationtests.MultiSheetsTest.testSheet2[KieBase type=CLOUD_IDENTITY]
Expecting value to be true but was false expected:<[tru]e> but was:<[fals]e>
org.drools.compiler.integrationtests.MultiSheetsTest.testSheet12[KieBase type=CLOUD_IDENTITY_MODEL_PATTERN] Unable to create KieModule, Errors Existed: [Message [id=1, kieBase=dtblaleKB, level=ERROR, path=org/drools/simple/candrink/CanDrink.drl.xls, line=5, column=0
text=Duplicate rule name: ID change_11], Message [id=2, kieBase=dtblaleKB, level=ERROR, path=org/drools/simple/candrink/CanDrink.drl.xls, line=13, column=0
text=Duplicate rule name: ID change_12]]
org.drools.compiler.integrationtests.MultiSheetsTest.testSheet2[KieBase type=CLOUD_IDENTITY_MODEL_PATTERN]
Expecting value to be true but was false expected:<[tru]e> but was:<[fals]e>
org.drools.compiler.integrationtests.incrementalcompilation.IncrementalCompilationTest.testDecisionTable[KieBase type=CLOUD_IDENTITY] Cannot find KieModule: org.kie:test-dtable:1.1.1
org.drools.compiler.integrationtests.incrementalcompilation.IncrementalCompilationTest.testDecisionTable[KieBase type=CLOUD_IDENTITY_MODEL_PATTERN] Cannot find KieModule: org.kie:test-dtable:1.1.1

private static final transient Logger logger = LoggerFactory.getLogger( DecisionTableProviderImpl.class );
private static final Logger logger = LoggerFactory.getLogger( DecisionTableProviderImpl.class );

private Map<String, String> compiledDtablesCache = new HashMap<>();
Copy link
Contributor

@tkobayas tkobayas Sep 24, 2024

Choose a reason for hiding this comment

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

Thank you, @mariofusco . Shouldn't this be ConcurrentHashMap because DecisionTableProviderImpl is a singleton in DecisionTableFactory?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess you're right. In reality I was thinking to it yesterday when I wrote that code, and I decided that we don't have any situation when we do 2 dtable -> drl conversions in parallel, but I was not properly considering that this is a singleton indeed. Better to play on the safe side on this. I will turn it into a ConcurrentHashMap, thanks.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@tkobayas done.

Copy link
Contributor

@tkobayas tkobayas left a comment

Choose a reason for hiding this comment

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

Thanks!

@mariofusco mariofusco merged commit 011e17a into apache:main Sep 24, 2024
7 of 10 checks passed
@mariofusco mariofusco deleted the d6088 branch September 24, 2024 09:45
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.

Performance issue with many kbase models and many large spreadsheets
3 participants