-
Notifications
You must be signed in to change notification settings - Fork 5.3k
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
Noexcept for hashing function objects #1480
Comments
@dogancan, do you have time to look at this? |
Not at the moment but I can look into it when I have the chance. That might take a while though. From a cursory glance, the idea is to annotate any function that cannot throw with noexcept, right? |
not any function, but specifically the () operators of the hashing-function
objects.
apparently there is a performance issue if you don't do that.
Either that, or I thought you might have some insight into whether that was
necessary or not.
…On Thu, Mar 23, 2017 at 8:27 PM, Dogan Can ***@***.***> wrote:
Not at the moment but I can look into it when I have the chance. That
might take a while though. From a cursory glance, the idea is to annotate
any function that cannot throw with noexcept, right?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#1480 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ADJVu4n3l3GsUKaYDtRYi7k2o5JFhciFks5row3egaJpZM4MVFgI>
.
|
Oh yeah that's what I meant to ask. There is some further info in the GCC manual and this blog post. TLDR; C++ standard requires erase and swap operations to be non-throwing. libc++ and libstdc++ implementations of unordered (multi)map/set need access to hash values for these operations. If hash function can throw, it cannot be called by these operations, hence hash values for all items need to be stored along with the items. If hash function is "fast", or if these operations are rare, then storing hash values is wasteful. |
OK, we should probably add nothrow then.
There may be hashing functions declared in various places but they should
all contain the string 'Hasher'.
Is there anyone who has time to create a PR for this? It should be fairly
mechanical.
…On Thu, Mar 23, 2017 at 9:30 PM, Dogan Can ***@***.***> wrote:
Oh yeah that's what I meant to ask. There is some further info in the GCC
manual
<https://gcc.gnu.org/onlinedocs/libstdc++/manual/unordered_associative.html>
and this blog post
<http://bannalia.blogspot.com/2013/10/implementation-of-c-unordered.html>.
TLDR; C++ standard requires erase and swap operations to be non-throwing.
libc++ and libstdc++ implementations of unordered (multi)map/set need
access to hash values for these operations. If hash function can throw, it
cannot be called by these operations, hence hash values for all items need
to be stored along with the items. If hash function is "fast", or if these
operations are rare, then storing hash values is wasteful.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#1480 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ADJVu2x1vCEWYRLB8Z9zRj-TeItNtQqFks5roxzPgaJpZM4MVFgI>
.
|
@tomkocse, do you have time for this? |
If Tom does not have time, I can look into it.
Y.
…On Mar 24, 2017 03:53, "Daniel Povey" ***@***.***> wrote:
@tomkocse <https://github.com/tomkocse>, do you have time for this?
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#1480 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AKisX2Z5FWeWfFBVFOZtcFUIdk8N_y5aks5rozAxgaJpZM4MVFgI>
.
|
I can start to do this on Monday. |
resolved in #1519 |
I've never really used noexcept, but apparently it's important for hashing functions:
http://stackoverflow.com/questions/13580823/sigfpe-when-accessing-unordered-map
I hope someone can look into this.
Kaldi has various hashing function objects, see stl-utils.h.
The text was updated successfully, but these errors were encountered: