You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kie.apache.org by "jmreardon-yoppworks (via GitHub)" <gi...@apache.org> on 2024/02/03 00:18:48 UTC
[PR] [incubator-kie-issues#875] DMN - Do not create an exception when property is not defined. [incubator-kie-drools]
jmreardon-yoppworks opened a new pull request, #5672:
URL: https://github.com/apache/incubator-kie-drools/pull/5672
This PR addresses this issue https://github.com/apache/incubator-kie-issues/issues/875 by removing the creation of an exception when a property is not defined.
This change is only observable by calling the `getValueResult`, but none of the uses of that method are affected by this change. There are only 4 uses of this method in the code base, and each case is either:
* guarded by a call to `isDefined` so that `getValueResult` is not called
* or transforms not defined results to `null` anyways.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@kie.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@kie.apache.org
For additional commands, e-mail: commits-help@kie.apache.org
Re: [PR] [incubator-kie-issues#875] DMN - Do not create an exception when property is not defined. [incubator-kie-drools]
Posted by "jmreardon-yoppworks (via GitHub)" <gi...@apache.org>.
jmreardon-yoppworks commented on PR #5672:
URL: https://github.com/apache/incubator-kie-drools/pull/5672#issuecomment-1935845388
> > @gitgabrio , I'm fine using a singleton of the `UnsupportedOperationException`. I started with the null approach because singleton exceptions are a little obscure, but the singleton is safer if we have downstream dependencies to worry about.
>
> Thanks @jmreardon-yoppworks ! Please let us know if that approach would solve the performance issue
I'll rerun my bench with it this weekend to verify. I would be shocked if it had a different profile from the null fix. The only expensive part was filling the trace and the singleton avoids that too.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@kie.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@kie.apache.org
For additional commands, e-mail: commits-help@kie.apache.org
Re: [PR] [incubator-kie-issues#875] DMN - Do not create an exception when property is not defined. [incubator-kie-drools]
Posted by "baldimir (via GitHub)" <gi...@apache.org>.
baldimir merged PR #5672:
URL: https://github.com/apache/incubator-kie-drools/pull/5672
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@kie.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@kie.apache.org
For additional commands, e-mail: commits-help@kie.apache.org
Re: [PR] [incubator-kie-issues#875] DMN - Do not create an exception when property is not defined. [incubator-kie-drools]
Posted by "baldimir (via GitHub)" <gi...@apache.org>.
baldimir commented on PR #5672:
URL: https://github.com/apache/incubator-kie-drools/pull/5672#issuecomment-1938235059
Thank you very much for the contribution. Looks good to me. When the PR checks are done and pass, it can be merged.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@kie.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@kie.apache.org
For additional commands, e-mail: commits-help@kie.apache.org
Re: [PR] [incubator-kie-issues#875] DMN - Do not create an exception when property is not defined. [incubator-kie-drools]
Posted by "jmreardon-yoppworks (via GitHub)" <gi...@apache.org>.
jmreardon-yoppworks commented on PR #5672:
URL: https://github.com/apache/incubator-kie-drools/pull/5672#issuecomment-1936815112
Static exception comes out slightly ahead of my null benchmark [but that could be my computer having fewer Intellij instances open today].
## Static Exception Benchmarks
```
[info] Benchmark Mode Cnt Score Error Units
[info] DmnListFilterBench.evaluate1Function thrpt 10 72.720 ± 0.278 ops/ms
[info] DmnListFilterBench.evaluate2Functions thrpt 10 49.537 ± 0.375 ops/ms
[info] DmnListFilterBench.evaluateBool thrpt 10 149.365 ± 0.891 ops/ms
[info] DmnListFilterBench.evaluateBoolItem thrpt 10 126.287 ± 0.452 ops/ms
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@kie.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@kie.apache.org
For additional commands, e-mail: commits-help@kie.apache.org
Re: [PR] [incubator-kie-issues#875] DMN - Do not create an exception when property is not defined. [incubator-kie-drools]
Posted by "jmreardon-yoppworks (via GitHub)" <gi...@apache.org>.
jmreardon-yoppworks commented on PR #5672:
URL: https://github.com/apache/incubator-kie-drools/pull/5672#issuecomment-1935800851
@gitgabrio , I'm fine using a singleton of the `UnsupportedOperationException`. I started with the null approach because singleton exceptions are a little obscure, but the singleton is safer if we have downstream dependencies to worry about.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@kie.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@kie.apache.org
For additional commands, e-mail: commits-help@kie.apache.org
Re: [PR] [incubator-kie-issues#875] DMN - Do not create an exception when property is not defined. [incubator-kie-drools]
Posted by "gitgabrio (via GitHub)" <gi...@apache.org>.
gitgabrio commented on PR #5672:
URL: https://github.com/apache/incubator-kie-drools/pull/5672#issuecomment-1935836577
> @gitgabrio , I'm fine using a singleton of the `UnsupportedOperationException`. I started with the null approach because singleton exceptions are a little obscure, but the singleton is safer if we have downstream dependencies to worry about.
Thanks @jmreardon-yoppworks ! Please let us know if that approach would solve the performance issue
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@kie.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@kie.apache.org
For additional commands, e-mail: commits-help@kie.apache.org
Re: [PR] [incubator-kie-issues#875] DMN - Do not create an exception when property is not defined. [incubator-kie-drools]
Posted by "gitgabrio (via GitHub)" <gi...@apache.org>.
gitgabrio commented on PR #5672:
URL: https://github.com/apache/incubator-kie-drools/pull/5672#issuecomment-1935637449
Hi @jmreardon-yoppworks
Firtst of all, thatnks again for your PR and your analysys on that.
I'm looking at your modification, and I agree with your observations about actual usage of the generated exception.
But, here's my small concerns.
On one side, the definition of the class clearly define the semantic of the `valueResult` as
`Either<Exception, Object>`
On the other side, this modification would introduce a small inconsistency, i.e. in some cases the `getDefinedValue` would return an` Either of righ(null) `, and in other cases an `Either of left (Exception)`.
Moreover, those methods are `public static`, so maybe used elsewhere, with the defined semantic.
What if, instead, we use a singleton of the `UnsupportedOperationException`, so that
1. the semantic of the class is preserved
2. the exception is created only once, so it should reduce the overall impact
Wdyt ?
FYI @baldimir @yesamer
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@kie.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@kie.apache.org
For additional commands, e-mail: commits-help@kie.apache.org