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

Delay Presto map hash table build when lookups are few #22587

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

pranjalssh
Copy link
Contributor

Add session property map_lookups_without_hashtable, which denotes how many map lookups will be done using linear search, before resorting to hash table build of the entire map. Default is 0.

Usually hashing cost is high enough that its best to not build the hash table for map objects. Only when number of lookups exceed some threshold, we should resort to build the hashtable. This change controls that using a session prop.

== RELEASE NOTES ==

General Changes
* Add session property `map_lookups_without_hashtable`, which denotes how many map lookups will be done using linear search, before resorting to hash table build of the entire map. Default is 0.

@pranjalssh pranjalssh requested a review from a team as a code owner April 22, 2024 22:23
return 2 * i + 1;
}
}
catch (Throwable throwable) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Per Effective Java, you should almost never catch Throwable. What's the more specific exception here?

return 2 * i + 1;
}
}
catch (Throwable throwable) {
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

return 2 * i + 1;
}
}
catch (Throwable throwable) {
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

return 2 * i + 1;
}
}
catch (Throwable throwable) {
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

return 2 * i + 1;
}
}
catch (Throwable throwable) {
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

@steveburnett
Copy link
Contributor

  • Please add documentation for the session property, and then I'll be happy to review this PR.

  • Thanks for writing a release note entry! Consider the following small suggestion adding your PR# in the entry to make easier the work of assembling the release note for the next release:

== RELEASE NOTES ==

General Changes
* Add session property `map_lookups_without_hashtable`, which denotes how many map lookups will be done using linear search, before resorting to hash table build of the entire map. Default is 0. :pr:`22587`

@aditi-pandit
Copy link
Contributor

@pranjalssh, @mbasmanova : Is there an equivalent for this in Prestissimo engine ? My intuition is that because of runtime optimizations in HashTable build we might not need this. But just thought to ask anyways.

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.

None yet

4 participants