Skip to content

Poor bucketing behavior in ThreadLocalStorage<T> (half the hash table goes unused) #491

Open
@ChrisGuzak

Description

This ThreadLocalStorage class in WIL is a hand-rolled TLS like thing that stores state on a per-thread basis. It boils down to a simple fixed size hash table with 10 buckets, where the thread to bucket mapping is (thread-ID mod 10). Search for m_hashArray in result.h to find the code in question.

The bug here is that half of the buckets in these hash tables are never utilized, because thread IDs are never odd.

Dump any instance of one of these things and you will always see that the odd slots in the table are unused.

0:000> dx shcore!wil::details_abi::g_pProcessLocalData->m_data->m_data.threads.m_hashArray
shcore!wil::details_abi::g_pProcessLocalData->m_data->m_data.threads.m_hashArray                 [Type: wil::details_abi::ThreadLocalStorage<wil::details_abi::ThreadLocalData>::Node * [10]]
    [0]              : 0x383f2d70 [Type: wil::details_abi::ThreadLocalStorage<wil::details_abi::ThreadLocalData>::Node *]
    [1]              : 0x0 [Type: wil::details_abi::ThreadLocalStorage<wil::details_abi::ThreadLocalData>::Node *]
    [2]              : 0x30104580 [Type: wil::details_abi::ThreadLocalStorage<wil::details_abi::ThreadLocalData>::Node *]
    [3]              : 0x0 [Type: wil::details_abi::ThreadLocalStorage<wil::details_abi::ThreadLocalData>::Node *]
    [4]              : 0x6cb3e10 [Type: wil::details_abi::ThreadLocalStorage<wil::details_abi::ThreadLocalData>::Node *]
    [5]              : 0x0 [Type: wil::details_abi::ThreadLocalStorage<wil::details_abi::ThreadLocalData>::Node *]
    [6]              : 0x300ee3a0 [Type: wil::details_abi::ThreadLocalStorage<wil::details_abi::ThreadLocalData>::Node *]
    [7]              : 0x0 [Type: wil::details_abi::ThreadLocalStorage<wil::details_abi::ThreadLocalData>::Node *]
    [8]              : 0x4515c9d0 [Type: wil::details_abi::ThreadLocalStorage<wil::details_abi::ThreadLocalData>::Node *]
    [9]              : 0x0 [Type: wil::details_abi::ThreadLocalStorage<wil::details_abi::ThreadLocalData>::Node *]

This would dump all instances in the process:

!for_each_module dx ${@#ModuleName}!*!wil::details_abi::g_pProcessLocalData->m_data->m_data.threads.m_hashArray

The low two bits for thread IDs seem to never be set. This code could would be better off shifting the thread ID right by two before doing the mod. All buckets would be used, and the length of the hash chains would be cut in half.
size_t const index = ((threadId >> 2) % ARRAYSIZE(m_hashArray));

I moved this from http://task.ms/51574898 here in case someone is willing to pick up this fix.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions