DROOLS-4735: [DMN Designer] Grid performance is dire #3004
Conversation
manstis
changed the title
Manual Review Note 1When I compare files produced by kie-wb-distributions/business-central-parent/business-central-webapp and kie-wb-common/kie-wb-common-dmn/kie-wb-common-dmn-webapp-kogito-testeting the second one doesn't produce new line character in the source file. Is it expected? Not sure if caused by this PR. |
|
Hi @jomarko this PR does not affect the UI to XML process at all so if client-side marshalling ( |
|
@manstis due to memory usage, should we keep client-site-marshalling this way (without new lines) or should I report a KOGITO issue? |
|
Hi @jomarko I doubt the inclusion of (even 1000's of) new line characters will affect memory consumption unduly. It's more than likely the XML library we use client-side is not adding the characters (and having them does make the XML more human readable). Feel free to make a KOGITO JIRA (IIRC @tiagobento formats the XML in VSCode so we might be able to use the same function to format the output of the client-side marshaller before writing to file). |
|
@manstis reported as https://issues.jboss.org/browse/KOGITO-616 |
|
@manstis see my manual acceptance test results at https://issues.jboss.org/browse/DROOLS-4735 I definitely see an improvement so I think I could approve, but the saving and editing by mouse are quite important, please share your opinion. |
|
@jomarko Hello, I left a comment on the JIRA regarding editing (i.e. I think a new JIRA is in order.. you can get the same result before this PR with larger tables). I think saving should also be a new JIRA as we'd need to see where the bottle neck is.. marshalling UI->XML or even XML->server. |
|
@manstis @jomarko I'm using native XSLT support on browsers to format the output from both DMN and BPMN editors on kogito-tooling extensions. You can see the rules we use here: https://github.com/kiegroup/kogito-tooling/blob/master/packages/kie-bc-editors/src/DefaultXmlFormatter.ts |
|
Reported remaining issues: |
|
@danielezonca All issues resolved/commented. Please re-review at your pleasure/leisure! |
|
Jenkins please execute full downstream build. |
| @@ -248,10 +249,6 @@ private OutputClauseColumn makeOutputClauseColumn(final int index, | |||
| return column; | |||
| } | |||
|
|
|||
| private GridRow makeDecisionTableRow() { | |||
| return new LiteralExpressionGridRow(getExpressionTextLineHeight(getRenderer().getTheme())); | |||
There was a problem hiding this comment.
Personally I would prefer to keep this factory method (even with a simple new statement) so it will be easier to change in the future. Btw no strong opinion
See https://issues.jboss.org/browse/DROOLS-4735
Part of an ensemble:
This PR moves calculation of a row's height from "on demand" calculation of cells' heights to "on value set" i.e. the height of a cell is now calculated when it's value is set hence moving much of the expensive calculation to a far less frequently invoked point in the codebase.
This PR also ensures DMN's
GridLayeris not instantiated (by Stunner) multiple times and subsequent event handlers attached to each instance (thus leading to multiple re-handling of events that slowed things down).