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

[Fix #3772] Duplicate function error #3774

Merged
merged 1 commit into from
Nov 13, 2024
Merged

Conversation

fjtirado
Copy link
Contributor

Fix #3772

items.forEach(item -> {
V toAdd = converter.apply(item);
if (!helper.add(toAdd)) {
duplicates.compute(toAdd, (k, v) -> v == null ? 2 : ++v);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Although the helper set is not stricly needed, since you can remove not duplicates from the map after the main loop is completed
duplicates().values().removeIf ( v -> v == 1);
it will slighly affects the performace of the most common case. You have to remove original number - duplicate elements from the map. If duplicate elements is 0 (the most common case) you remove the original number.
It can be argued thad adding the helper Set consumes more memory, but this is not completely true, because at the end a HashSet is a "capped" HashMap. Since the number of Map key buckets depends on the number of duplicates, with a helper map you have original number + duplicate number buckets, without it you have original number buckets. So you only have the duplicate buckets as additional memory (precisely the addtional memory that will be preserved when the method returns) and as benefit you save the removal.

{
"name": "pushData",
"type": "custom",
"operation": "script:python:print('javierito')"
Copy link
Contributor

Choose a reason for hiding this comment

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

Different operation but same name (which is the one to be checked) makes them duplicated, clear example!

Copy link
Contributor

@gmunozfe gmunozfe left a comment

Choose a reason for hiding this comment

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

Looks good to me, nice implementation, taking care for the best performance!

@fjtirado fjtirado merged commit 34a4323 into apache:main Nov 13, 2024
5 of 6 checks passed
fjtirado added a commit to fjtirado/kogito-runtimes that referenced this pull request Nov 13, 2024
fjtirado added a commit to fjtirado/kogito-runtimes that referenced this pull request Nov 13, 2024
rgdoliveira pushed a commit to kiegroup/kogito-runtimes that referenced this pull request Nov 13, 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.

Duplicated function in workflow definition is ignored without message
3 participants