ywkaras commented on a change in pull request #7382: URL: https://github.com/apache/trafficserver/pull/7382#discussion_r540474285
########## File path: include/tscore/ver_ptr.h ########## @@ -0,0 +1,288 @@ +/** @file + + Provides a "versioned pointer" data type. + + @section license License + + Licensed to the Apache Software Foundation (ASF) under one + or more contributor license agreements. See the NOTICE file + distributed with this work for additional information + regarding copyright ownership. The ASF licenses this file + to you under the Apache License, Version 2.0 (the + "License"); you may not use this file except in compliance + with the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. + */ + +#pragma once + +#include <cstdint> +#include <cstring> +#include <atomic> + +namespace ts +{ +namespace detail +{ + namespace versioned_ptr + { + // This is true if pointers are 64 bits but the two most significant bytes are not used, except for the + // most significant bit. + // + bool const Ptr64bits15unused = +#if defined(__x86_64__) || defined(__ia64__) || defined(__powerpc64__) || defined(__aarch64__) || defined(__mips64) + true +#else + false +#endif + ; + +#if TS_HAS_128BIT_CAS + + bool const CAE128 = true; + + using Bits128_type = __int128_t; + + static_assert(sizeof(Bits128_type) == 16); + + // This static assert is commented out because it fails. It seems that our Automake voodoo sets + // TS_HAS_128BIT_CAS to 1 even if there is no lock-free 128-bit CAS available. See + // https://godbolt.org/z/ncYsv9 . Looking at the generated assembly code, the 128-bit CAS operations + // cause calls into the run-time library, which may mean that are not actually lock-free. + // See also + // + // static_assert(std::atomic<Bits128_type>::is_always_lock_free); + +#else + + bool const CAE128 = false; + + using Bits128_type = char; // Not used. + +#endif + + struct VPS { + void *ptr; + unsigned version; + }; + + inline bool + operator==(VPS const &a, VPS const &b) + { + return (a.ptr == b.ptr) && (a.version == b.version); + } + + template <typename AAT, bool Version_in_ptr> class Type_ + { + public: + using Atomic_access_type = AAT; + + explicit Type_(void *p = nullptr, unsigned v = 0) + { + _vp.ptr = p; + _vp.version = v; + + const std::size_t Pad = sizeof(_aa) - sizeof(_vp); + if (Pad) { + // Make sure pad bytes are consistently zero. + // + std::memset(reinterpret_cast<char *>(&_vp) + sizeof(_vp), 0, Pad); + } + } + + explicit Type_(Atomic_access_type const &aa) { _aa = aa; } + + void * + ptr() const + { + return _vp.ptr; + } + + void + ptr(void *p) + { + _vp.ptr = p; + } + + unsigned + version() const + { + return _vp.version; + } + + void + version(unsigned v) + { + _vp.version = v; + } + + private: + union { + VPS _vp; + AAT _aa; + }; + + static_assert(sizeof(_vp) <= sizeof(_aa)); + }; + + namespace + { + unsigned const Version_bits = 15; + unsigned const Version_shift = 48; + constexpr unsigned + Mask_version(unsigned v) + { + return v & ((1U << (Version_bits + 1)) - 1); + } + std::uint64_t const Version_mask = static_cast<std::uint64_t>(Mask_version(~0U)) << Version_shift; + } // namespace + + template <> class Type_<std::uint64_t, true> + { + public: + using Atomic_access_type = std::uint64_t; + + Type_(){}; + + explicit Type_(void *p, unsigned v = 0) + { + _ptr = reinterpret_cast<std::uint64_t>(p) & ~Version_mask; + _ptr |= static_cast<uint64_t>(Mask_version(v)) << Version_shift; + } + + explicit Type_(Atomic_access_type const &ptr) { _ptr = ptr; } + + void * + ptr() const + { + return reinterpret_cast<void *>(_ptr & ~Version_mask); + } + + void + ptr(void *p) + { + _ptr &= Version_mask; + _ptr |= reinterpret_cast<std::uint64_t>(p) & ~Version_mask; + } + + unsigned + version() const + { + return Mask_version(static_cast<unsigned>(_ptr >> Version_shift)); + } + + void + version(unsigned v) + { + _ptr &= ~Version_mask; + _ptr |= static_cast<std::uint64_t>(Mask_version(v)) << Version_shift; + } + + private: + std::uint64_t _ptr; + }; + + enum Impl_type { + DEFAULT, // Atomic access may not be lock free. + VERSION_IN_PTR, // Use bits inside of 64-bit pointer to store version. + BITS64, // VPS structure is less than 64 bits, and 64-bit access is lock free. + BITS128 // VPS structure is less than 128 bits, and 128-bit compare-and-exchange is available. + }; + + constexpr Impl_type + Impl() + { + if (std::atomic<std::uint64_t>::is_always_lock_free) { + if (sizeof(VPS) <= 8) { + return BITS64; + + } else if (Ptr64bits15unused && (sizeof(void *) == 8)) { + return VERSION_IN_PTR; Review comment: Using the stolen pointer bits approach, with my new implementation, a regression test fails: ``` Regression test(PVC) still in progress Regression test(HostDBProcessor) still in progress Regression test(HostDBTests) still in progress Regression test(PVC) still in progress Regression test(HostDBProcessor) still in progress Regression test(HostDBTests) still in progress Regression test(PVC) still in progress Regression test(HostDBProcessor) still in progress Regression test(HostDBTests) still in progress Regression test(PVC) still in progress Regression test(HostDBProcessor) still in progress Regression test(HostDBTests) still in progress Regression test(PVC) still in progress Regression test(HostDBProcessor) still in progress Regression test(HostDBTests) still in progress Regression test(PVC) still in progress Regression test(HostDBProcessor) still in progress Regression test(HostDBTests) still in progress Regression test(PVC) still in progress Regression test(HostDBProcessor) still in progress Regression test(HostDBTests) still in progress Regression test(PVC) still in progress Regression test(HostDBProcessor) still in progress Regression test(HostDBTests) still in progress Regression test(PVC) still in progress Regression test(HostDBProcessor) still in progress Regression test(HostDBTests) still in progress Regression test(PVC) still in progress Regression test(HostDBProcessor) still in progress Regression test(HostDBTests) still in progress Regression test(PVC) still in progress Regression test(HostDBProcessor) still in progress Regression test(HostDBTests) still in progress Regression test(PVC) still in progress Regression test(HostDBProcessor) still in progress Regression test(HostDBTests) still in progress Regression test(PVC) still in progress Regression test(HostDBProcessor) still in progress Regression test(HostDBTests) still in progress Regression test(PVC) still in progress Regression test(HostDBProcessor) still in progress Regression test(HostDBTests) still in progress Regression test(PVC) still in progress Regression test(HostDBProcessor) still in progress Regression test(HostDBTests) still in progress Regression test(PVC) still in progress Regression test(HostDBProcessor) still in progress Regression test(HostDBTests) still in progress Regression test(PVC) still in progress Regression test(HostDBProcessor) still in progress Regression test(HostDBTests) still in progress Regression test(PVC) still in progress Regression test(HostDBProcessor) still in progress Regression test(HostDBTests) still in progress Regression test(PVC) still in progress Regression test(HostDBProcessor) still in progress Regression test(HostDBTests) still in progress Regression test(PVC) still in progress Regression test(HostDBProcessor) still in progress Regression test(HostDBTests) still in progress Regression test(PVC) still in progress RPRINT HostDBTests: HostDBTestReverse: reversed static-css-csq-225018.business.bouyguestelecom.com Regression test(HostDBProcessor) still in progress Regression test(HostDBTests) still in progress Regression test(PVC) still in progress Regression test(HostDBProcessor) still in progress Regression test(HostDBTests) still in progress Regression test(PVC) still in progress Regression test(HostDBProcessor) still in progress Regression test(HostDBTests) still in progress Regression test(PVC) still in progress RPRINT HostDBTests: HostDBTestReverse: reversed apn-77-114-28-193.dynamic.gprs.plus.pl RPRINT HostDBTests: HostDBTestReverse: reversed 59-113-175-47.dynamic-ip.hinet.net Regression test(HostDBProcessor) still in progress Regression test(HostDBTests) still in progress Regression test(PVC) still in progress RPRINT HostDBTests: HostDBTestReverse: reversed 110-132-7-105.rev.home.ne.jp Regression test(HostDBProcessor) still in progress Regression test(HostDBTests) still in progress Regression test(PVC) still in progress Regression test(HostDBProcessor) still in progress Regression test(HostDBTests) still in progress Regression test(PVC) still in progress RPRINT HostDBTests: HostDBTestReverse: reversed 165.sub-97-204-116.myvzw.com RPRINT HostDBTests: HostDBTestReverse: reversed cpe-66-66-7-152.rochester.res.rr.com Regression test(HostDBProcessor) still in progress Regression test(HostDBTests) still in progress Regression test(PVC) still in progress RPRINT HostDBTests: HostDBTestReverse: reversed host88.fibrewired.on.ca Regression test(HostDBProcessor) still in progress Regression test(HostDBTests) still in progress Regression test(PVC) still in progress RPRINT HostDBProcessor: hostdbinfo r=a49f9780 RPRINT HostDBProcessor: hostdbinfo hostname=www.ibm.com RPRINT HostDBProcessor: hostdbinfo rr 0 RPRINT HostDBProcessor: hostdbinfo A ip=23.47.163.207 RPRINT HostDBTests: HostDBTestReverse: reversed ec2-35-166-27-40.us-west-2.compute.amazonaws.com RPRINT HostDBProcessor: hostdbinfo r=a49f9700 RPRINT HostDBProcessor: hostdbinfo hostname=www.microsoft.com RPRINT HostDBProcessor: hostdbinfo rr 0 RPRINT HostDBProcessor: hostdbinfo A ip=23.37.117.182 Regression test(HostDBProcessor) still in progress Regression test(HostDBTests) still in progress Regression test(PVC) still in progress RPRINT HostDBProcessor: hostdbinfo r=a49f9680 RPRINT HostDBProcessor: hostdbinfo hostname=www.coke.com RPRINT HostDBProcessor: hostdbinfo rr 0 RPRINT HostDBProcessor: hostdbinfo A ip=104.121.207.153 RPRINT HostDBProcessor: hostdbinfo r=a49f9600 RPRINT HostDBProcessor: hostdbinfo hostname=(null) RPRINT HostDBProcessor: hostdbinfo rr 0 RPRINT HostDBProcessor: hostdbinfo A ip=4.2.2.2 RPRINT HostDBProcessor: hostdbinfo r=a49f9580 RPRINT HostDBProcessor: hostdbinfo hostname=(null) RPRINT HostDBProcessor: hostdbinfo rr 0 RPRINT HostDBProcessor: hostdbinfo A ip=127.0.0.1 RPRINT HostDBProcessor: HostDBTestRR: 0 outstanding 5 success 1 failure RPRINT HostDBTests: HostDBTestReverse: reversed 108-198-194-205.lightspeed.jcsnms.sbcglobal.net RPRINT HostDBTests: HostDBTestReverse: reversed host137-66-252-76.netaspx.com REGRESSION_RESULT HostDBProcessor: FAILED ``` ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [email protected]
