Yingchun Lai has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14555 )

Change subject: KUDU-2977 Sharding block map to speed up tserver startup
......................................................................


Patch Set 5:

> Patch Set 5:
>
> The TSAN failure looks real but I don't know what's causing it.

I read some spp.h code, found that it's a problem of spare-map. Code fragment 
below (thirdparty/installed/common/include/sparsepp/spp.h:1082):
    static uint32_t _sizing(uint32_t n)
    {
#if !defined(SPP_ALLOC_SZ) || (SPP_ALLOC_SZ == 0)
        // aggressive allocation first, then decreasing as sparsegroups fill up
        // --------------------------------------------------------------------
        static uint8_t s_alloc_batch_sz[SPP_GROUP_SIZE] = { 0 };
        if (!s_alloc_batch_sz[0])
        {
            // 32 bit bitmap
            // ........ .... .... .. .. .. .. .  .  .  .  .  .  .  .
            //     8     12   16  18 20 22 24 25 26   ...          32
            // ------------------------------------------------------
            uint8_t group_sz          = SPP_GROUP_SIZE / 4;
            uint8_t group_start_alloc = SPP_GROUP_SIZE / 8; //4;
            uint8_t alloc_sz          = group_start_alloc;
            for (int i=0; i<4; ++i)
            {
                for (int j=0; j<group_sz; ++j)
                {
                    if (j && j % group_start_alloc == 0)
                        alloc_sz += group_start_alloc;
                    s_alloc_batch_sz[i * group_sz + j] = alloc_sz;    // data 
race here!!!
                }
                if (group_start_alloc > 2)
                    group_start_alloc /= 2;
                alloc_sz += group_start_alloc;
            }
        }

        return n ? static_cast<uint32_t>(s_alloc_batch_sz[n-1]) : 0; // more 
aggressive alloc at the beginning

#elif (SPP_ALLOC_SZ == 1)
        // use as little memory as possible - slowest insert/delete in table
        // -----------------------------------------------------------------
        return n;
#else
        // decent compromise when SPP_ALLOC_SZ == 2
        // ----------------------------------------
        static size_type sz_minus_1 = SPP_ALLOC_SZ - 1;
        return (n + sz_minus_1) & ~sz_minus_1;
#endif
    }

As s_alloc_batch_sz is static, and the code logic show that s_alloc_batch_sz 
will be init only once by 'if (!s_alloc_batch_sz[0])' block, duplicate calls 
has no effect.
So init block should be wrapped  by 'std::call_once' or something like that.


--
To view, visit http://gerrit.cloudera.org:8080/14555
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If0d5c13e051a2c1d6cfd1c9ad7db8a3cd195459d
Gerrit-Change-Number: 14555
Gerrit-PatchSet: 5
Gerrit-Owner: Yingchun Lai <405403...@qq.com>
Gerrit-Reviewer: Adar Dembo <a...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yingchun Lai <405403...@qq.com>
Gerrit-Comment-Date: Tue, 05 Nov 2019 10:03:08 +0000
Gerrit-HasComments: No

Reply via email to