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

prefetch first matched value #2

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Conversation

seckcoder
Copy link

Added the function to allow us prefetching the first matched value in f14 table. This can be helpful when used with F14VectorMap, where we can have cache miss when accessing the data vector.

yashNaN and others added 8 commits February 9, 2021 08:51
Summary:
F14 map `iterator` doesn't define `operator==` or `operator!=` to itself.
Those member functions take `const_iterator` as the right-hand side,
relying on the ability to convert implicitly from `iterator` to
`const_iterator`. This interacts poorly with c++20's default equality
operators, triggering clang's `-Wambiguous-reversed-operator`.

This diff replaces the equality member functions with friends that use
the same left-hand and right-hand types.  iterator != const_iterator
will now compile by implicitly converting the left-hand-side and then
calling `operator!=(const_iterator const&, const_iterator const&)`,
rather than calling `iterator::operator!=(const_iterator const&)`.
This resolves the ambiguity in c++20, and should work fine on earlier
versions, although I have not tested it on anything except c++17.

Test Plan:
does it build?
…tion

Summary:
If sigaltstack is in use, the alternate stack may be too small to
run the normal SafeStackTracePrinter. In that case we must fall back
to UnsafeSelfAllocateStackTracePrinter. Logic to enable this fallback
already exists, but the threshold of 8931 bytes is not sufficient
to run the current symbolizer code--the actual stack size needed by
SafeStackTracePrinter is greater than 48K.

GCC sanitizers use a sigaltstack of 4*SIGSTKSZ = 32K.
`Dwarf::findLocation` makes space for locals with `sub $0xa198,%rsp`,
which is 41368 bytes. Adjusting the alternate stack size on x86_64,
I observed a failure at 48K and a success at 56K.

Test Plan:
1. observe SIGSEGV during symbolization of SIGABRT in a GCC ASAN binary
2. adjust sigaltstack size to find a value that is (at least sometimes) big enough
3. increase threshold in folly, observe no SIGSEGV in the GCC ASAN binary
Summary:
Clang's -Wambiguous-reversed-operator warning claims that dynamic's and
IOBuf's iterators have an ambiguous operator== under C++20 rules. Language
lawyers will have to decide whether this is actually an issue or an
overly-aggressive warning that doesn't understand CRTP.  Either way,
a using declaration seems to be sufficient to resolve things.

Test Plan:
Build with clang-12 and -std=c++20
Speed up findLocation in the absence of .debug_aranges
Copy link

@benhannel benhannel left a comment

Choose a reason for hiding this comment

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

Very nice!

auto hp = splitHash(this->computeKeyHash(key));
std::size_t index = hp.first;
ChunkPtr chunk = chunks_ + (index & chunkMask_);
auto hits = chunk->tagMatchIter(hp.second);

Choose a reason for hiding this comment

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

Did you observe any overhead from doing tagMatchIter twice for every key?

nbronson pushed a commit that referenced this pull request Nov 21, 2022
Summary:
TSAN aborts on a data-race of the form:
```
WARNING: ThreadSanitizer: data race (pid=978596)
  Write of size 8 at 0x7b0400000b40 by thread T2:
    #0 operator delete(void*, unsigned long) <null>
    #1 folly::coro::timed_wait</*...*/>(/*...*/)::'lambda'(/*...*/)::operator()</*...*/>(/*...*/) const folly/experimental/coro/TimedWait.h:51
  Previous atomic write of size 1 at 0x7b0400000b40 by thread T1:
    #0 __tsan_atomic8_exchange <null>
    #1 std::atomic<bool>::exchange(bool, std::memory_order) libgcc/include/c++/bits/atomic_base.h:499
    #2 folly::coro::timed_wait</*...*/>(/*...*/)::'lambda'(/*...*/)::operator()</*...*/>(/*...*/) const folly/experimental/coro/TimedWait.h:64
```

Reviewed By: davidtgoldblatt, iahs

Differential Revision: D36159450

fbshipit-source-id: 860fcbbc1f689f9e166934f75a2ef60abdcad421
nbronson pushed a commit that referenced this pull request May 26, 2023
Summary:
AsyncUDPSocket test cases are crashing when running under llvm15. During
debugging it seems that the issue is the fact that the code tries to allocated 0
size array. Changing the code to prevent such allocation.

This is not very clean why to fix, but I am not sure if there is better one.
Please let me know if you have any suggestions.

Sample crash:
```
$ buck test //folly/io/async/test:async_udp_socket_test
...
stdout:
Note: Google Test filter = AsyncUDPSocketTest.TestDetachAttach
[==========] Running 1 test from 1 test suite.
[----------] Global test environment set-up.
[----------] 1 test from AsyncUDPSocketTest
[ RUN      ] AsyncUDPSocketTest.TestDetachAttach

stderr:
fbcode/folly/io/async/AsyncUDPSocket.cpp:699:10: runtime error: variable length array bound evaluates to non-positive value 0
    #0 0x7f4d8ed93704 in folly::AsyncUDPSocket::writev(folly::SocketAddress const&, iovec const*, unsigned long, int) fbcode/folly/io/async/AsyncUDPSocket.cpp:698
    #1 0x7f4d8ed9081f in folly::AsyncUDPSocket::writeGSO(folly::SocketAddress const&, std::unique_ptr<folly::IOBuf, std::default_delete<folly::IOBuf>> const&, int) fbcode/folly/io/async/AsyncUDPSocket.cpp:528
    #2 0x7f4d8ed900b2 in folly::AsyncUDPSocket::write(folly::SocketAddress const&, std::unique_ptr<folly::IOBuf, std::default_delete<folly::IOBuf>> const&) fbcode/folly/io/async/AsyncUDPSocket.cpp:660
    #3 0x350a05 in AsyncUDPSocketTest_TestDetachAttach_Test::TestBody() fbcode/folly/io/async/test/AsyncUDPSocketTest.cpp:914
    #4 0x7f4d90dd1ad5 in testing::Test::Run() /home/engshare/third-party2/googletest/1.11.0/src/googletest/googletest/src/gtest.cc:2682:50
    #5 0x7f4d90dd1ad5 in testing::Test::Run() /home/engshare/third-party2/googletest/1.11.0/src/googletest/googletest/src/gtest.cc:2672:6
    facebook#6 0x7f4d90dd1c64 in testing::TestInfo::Run() /home/engshare/third-party2/googletest/1.11.0/src/googletest/googletest/src/gtest.cc:2861:14
    facebook#7 0x7f4d90dd1c64 in testing::TestInfo::Run() /home/engshare/third-party2/googletest/1.11.0/src/googletest/googletest/src/gtest.cc:2833:6
    facebook#8 0x7f4d90dd2321 in testing::TestSuite::Run() /home/engshare/third-party2/googletest/1.11.0/src/googletest/googletest/src/gtest.cc:3015:31
    facebook#9 0x7f4d90dd2321 in testing::TestSuite::Run() /home/engshare/third-party2/googletest/1.11.0/src/googletest/googletest/src/gtest.cc:2993:6
    facebook#10 0x7f4d90dd2b1e in testing::internal::UnitTestImpl::RunAllTests() /home/engshare/third-party2/googletest/1.11.0/src/googletest/googletest/src/gtest.cc:5855:47
    facebook#11 0x7f4d90dd1d87 in bool testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) /home/engshare/third-party2/googletest/1.11.0/src/googletest/googletest/src/gtest.cc:2665:29
    facebook#12 0x7f4d90dd1d87 in testing::UnitTest::Run() /home/engshare/third-party2/googletest/1.11.0/src/googletest/googletest/src/gtest.cc:5438:55
    facebook#13 0x7f4d90d5c990 in RUN_ALL_TESTS() fbcode/third-party-buck/platform010/build/googletest/include/gtest/gtest.h:2490
    facebook#14 0x7f4d90d5c618 in main fbcode/common/gtest/LightMain.cpp:20
    facebook#15 0x7f4d8ea2c656 in __libc_start_call_main /home/engshare/third-party2/glibc/2.34/src/glibc-2.34/csu/../sysdeps/nptl/libc_start_call_main.h:58:16
    facebook#16 0x7f4d8ea2c717 in __libc_start_main@GLIBC_2.2.5 /home/engshare/third-party2/glibc/2.34/src/glibc-2.34/csu/../csu/libc-start.c:409:3
    facebook#17 0x33ea60 in _start /home/engshare/third-party2/glibc/2.34/src/glibc-2.34/csu/../sysdeps/x86_64/start.S:116
...
```

Reviewed By: russoue, dmm-fb

Differential Revision: D43858875

fbshipit-source-id: 93749bab17027b6dfc0dbc01b6c183e501a5494c
@nbronson nbronson force-pushed the master branch 3 times, most recently from 766f360 to ce3a748 Compare May 31, 2023 01:25
igorcanadi pushed a commit that referenced this pull request May 28, 2024
Summary:
ThreadLocalDetailTest.MultiThreadedTest had a data race on std::vector because multiple threads were accessing and modifying the std::vector without a mutex. I update the code to use indexes such that the vector is not updated concurrently.

Previous run failure that this fixes:

```
stderr:
==================
WARNING: ThreadSanitizer: data race (pid=1249005)
  Write of size 8 at 0x7fff195936d0 by main thread:
    #0 std::vector<std::unique_ptr<folly::test::Barrier, std::default_delete<folly::test::Barrier>>, std::allocator<std::unique_ptr<folly::test::Barrier, std::default_delete<folly::test::Barrier>>>>::pop_back() fbcode/third-party-buck/platform010/build/libgcc/include/c++/trunk/bits/stl_vector.h:1228 (thread_local_detail_test+0x116c28)
    #1 folly::threadlocal_detail::ThreadLocalDetailTest_MultiThreadedTest_Test::TestBody() fbcode/folly/detail/test/ThreadLocalDetailTest.cpp:121 (thread_local_detail_test+0x101aba)
    #2 void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) fbsource/src/gtest.cc:2675 (libthird-party_googletest_1.14.0_gtest.so+0xacf3c)
    #3 testing::Test::Run() fbsource/src/gtest.cc:2692 (libthird-party_googletest_1.14.0_gtest.so+0xacb9c)
    #4 testing::TestInfo::Run() fbsource/src/gtest.cc:2841 (libthird-party_googletest_1.14.0_gtest.so+0xafa4a)
    #5 testing::TestSuite::Run() fbsource/src/gtest.cc:3020 (libthird-party_googletest_1.14.0_gtest.so+0xb4303)
    facebook#6 testing::internal::UnitTestImpl::RunAllTests() fbsource/src/gtest.cc:5925 (libthird-party_googletest_1.14.0_gtest.so+0xd105e)
    facebook#7 bool testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) fbsource/src/gtest.cc:2675 (libthird-party_googletest_1.14.0_gtest.so+0xd08f1)
    facebook#8 testing::UnitTest::Run() fbsource/src/gtest.cc:5489 (libthird-party_googletest_1.14.0_gtest.so+0xd037e)
    facebook#9 RUN_ALL_TESTS() fbsource/gtest/gtest.h:2317 (libcommon_gtest_light_main.so+0x22a7)
    facebook#10 main fbcode/common/gtest/LightMain.cpp:20 (libcommon_gtest_light_main.so+0x2131)

  Previous read of size 8 at 0x7fff195936d0 by thread T123:
    #0 std::vector<std::unique_ptr<folly::test::Barrier, std::default_delete<folly::test::Barrier>>, std::allocator<std::unique_ptr<folly::test::Barrier, std::default_delete<folly::test::Barrier>>>>::size() const fbcode/third-party-buck/platform010/build/libgcc/include/c++/trunk/bits/stl_vector.h:919 (thread_local_detail_test+0x11b0bd)
    #1 std::vector<std::unique_ptr<folly::test::Barrier, std::default_delete<folly::test::Barrier>>, std::allocator<std::unique_ptr<folly::test::Barrier, std::default_delete<folly::test::Barrier>>>>::operator[](unsigned long) fbcode/third-party-buck/platform010/build/libgcc/include/c++/trunk/bits/stl_vector.h:1045 (thread_local_detail_test+0x11d101)
    #2 folly::threadlocal_detail::ThreadLocalDetailTest_MultiThreadedTest_Test::TestBody()::$_0::operator()() const fbcode/folly/detail/test/ThreadLocalDetailTest.cpp:97 (thread_local_detail_test+0x11d08c)
    #3 void std::__invoke_impl<void, folly::threadlocal_detail::ThreadLocalDetailTest_MultiThreadedTest_Test::TestBody()::$_0>(std::__invoke_other, folly::threadlocal_detail::ThreadLocalDetailTest_MultiThreadedTest_Test::TestBody()::$_0&&) fbcode/third-party-buck/platform010/build/libgcc/include/c++/trunk/bits/invoke.h:61 (thread_local_detail_test+0x11cf95)
    #4 std::__invoke_result<folly::threadlocal_detail::ThreadLocalDetailTest_MultiThreadedTest_Test::TestBody()::$_0>::type std::__invoke<folly::threadlocal_detail::ThreadLocalDetailTest_MultiThreadedTest_Test::TestBody()::$_0>(folly::threadlocal_detail::ThreadLocalDetailTest_MultiThreadedTest_Test::TestBody()::$_0&&) fbcode/third-party-buck/platform010/build/libgcc/include/c++/trunk/bits/invoke.h:96 (thread_local_detail_test+0x11cf05)
    #5 void std::thread::_Invoker<std::tuple<folly::threadlocal_detail::ThreadLocalDetailTest_MultiThreadedTest_Test::TestBody()::$_0>>::_M_invoke<0ul>(std::_Index_tuple<0ul>) fbcode/third-party-buck/platform010/build/libgcc/include/c++/trunk/bits/std_thread.h:253 (thread_local_detail_test+0x11cebd)
    facebook#6 std::thread::_Invoker<std::tuple<folly::threadlocal_detail::ThreadLocalDetailTest_MultiThreadedTest_Test::TestBody()::$_0>>::operator()() fbcode/third-party-buck/platform010/build/libgcc/include/c++/trunk/bits/std_thread.h:260 (thread_local_detail_test+0x11ce65)
    facebook#7 std::thread::_State_impl<std::thread::_Invoker<std::tuple<folly::threadlocal_detail::ThreadLocalDetailTest_MultiThreadedTest_Test::TestBody()::$_0>>>::_M_run() fbcode/third-party-buck/platform010/build/libgcc/include/c++/trunk/bits/std_thread.h:211 (thread_local_detail_test+0x11cd79)
    facebook#8 execute_native_thread_routine /home/engshare/third-party2/libgcc/11.x/src/gcc-11.x/x86_64-facebook-linux/libstdc++-v3/src/c++11/../../../.././libstdc++-v3/src/c++11/thread.cc:82:18 (libstdc++.so.6+0xdf4e4) (BuildId: 452d1cdae868baeeb2fdf1ab140f1c219bf50c6e)

  Location is stack of main thread.

  Location is global '??' at 0x7fff19575000 ([stack]+0x1e6d0)

  Thread T123 (tid=1249233, running) created by main thread at:
    #0 pthread_create <null> (thread_local_detail_test+0x156bef)
    #1 __gthread_create /home/engshare/third-party2/libgcc/11.x/src/gcc-11.x/x86_64-facebook-linux/libstdc++-v3/include/x86_64-facebook-linux/bits/gthr-default.h:663:35 (libstdc++.so.6+0xdf80e) (BuildId: 452d1cdae868baeeb2fdf1ab140f1c219bf50c6e)
    #2 std::thread::_M_start_thread(std::unique_ptr<std::thread::_State, std::default_delete<std::thread::_State>>, void (*)()) /home/engshare/third-party2/libgcc/11.x/src/gcc-11.x/x86_64-facebook-linux/libstdc++-v3/src/c++11/../../../.././libstdc++-v3/src/c++11/thread.cc:147:37 (libstdc++.so.6+0xdf80e)
    #3 folly::threadlocal_detail::ThreadLocalDetailTest_MultiThreadedTest_Test::TestBody() fbcode/folly/detail/test/ThreadLocalDetailTest.cpp:93 (thread_local_detail_test+0x1015cd)
    #4 void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) fbsource/src/gtest.cc:2675 (libthird-party_googletest_1.14.0_gtest.so+0xacf3c)
    #5 testing::Test::Run() fbsource/src/gtest.cc:2692 (libthird-party_googletest_1.14.0_gtest.so+0xacb9c)
    facebook#6 testing::TestInfo::Run() fbsource/src/gtest.cc:2841 (libthird-party_googletest_1.14.0_gtest.so+0xafa4a)
    facebook#7 testing::TestSuite::Run() fbsource/src/gtest.cc:3020 (libthird-party_googletest_1.14.0_gtest.so+0xb4303)
    facebook#8 testing::internal::UnitTestImpl::RunAllTests() fbsource/src/gtest.cc:5925 (libthird-party_googletest_1.14.0_gtest.so+0xd105e)
    facebook#9 bool testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) fbsource/src/gtest.cc:2675 (libthird-party_googletest_1.14.0_gtest.so+0xd08f1)
    facebook#10 testing::UnitTest::Run() fbsource/src/gtest.cc:5489 (libthird-party_googletest_1.14.0_gtest.so+0xd037e)
    facebook#11 RUN_ALL_TESTS() fbsource/gtest/gtest.h:2317 (libcommon_gtest_light_main.so+0x22a7)
    facebook#12 main fbcode/common/gtest/LightMain.cpp:20 (libcommon_gtest_light_main.so+0x2131)

ThreadSanitizer: data race fbcode/third-party-buck/platform010/build/libgcc/include/c++/trunk/bits/stl_vector.h:1228 in std::vector<std::unique_ptr<folly::test::Barrier, std::default_delete<folly::test::Barrier>>, std::allocator<std::unique_ptr<folly::test::Barrier, std::default_delete<folly::test::Barrier>>>>::pop_back()
==================
ThreadSanitizer: reported 1 warnings
```

Reviewed By: yfeldblum

Differential Revision: D56643303

fbshipit-source-id: cc364817ae79bc6418cc07faa35e1bcd21830c66
mpatou-openai pushed a commit that referenced this pull request Oct 16, 2024
Summary: detail::UnrollStep should go away, because otherwise it would be exposed to the user of `simdForEach`.

Reviewed By: yfeldblum

Differential Revision: D60587842

fbshipit-source-id: 34c75dcbb6ddeefeeae55c98f0656f7008d57e0e
mpatou-openai pushed a commit that referenced this pull request Oct 16, 2024
Summary:
Ucache with io_uring is crashing with this stack trace:

```
(lldb) bt
* thread #1, name = 'ucache', stop reason = signal SIGSEGV: address not mapped to object
  * frame #0: 0x0000000000000000
    frame #1: 0x00007f517ba44560 libc.so.6`__restore_rt at libc_sigaction.c:13
    frame #2: 0x000000000a16e249 ucache`folly::AsyncIoUringSocket::ReadSqe::callback(this=0x00007f498c5955e0, cqe=<unavailable>) at AsyncIoUringSocket.cpp:639
    frame #3: 0x000000000964196a ucache`folly::IoUringBackend::getActiveEvents(folly::IoUringBackend::WaitForEventsMode) [inlined] folly::IoSqeBase::internalCallback(this=<unavailable>, cqe=<unavailable>) at IoUringBackend.cpp:0
    frame #4: 0x000000000964195c ucache`folly::IoUringBackend::getActiveEvents(folly::IoUringBackend::WaitForEventsMode) [inlined] folly::IoUringBackend::internalProcessCqe(this=0x00007f4a8f989c00, maxGet=4294967295, mode=NORMAL) at IoUringBackend.cpp:1556
    frame #5: 0x000000000964190f ucache`folly::IoUringBackend::getActiveEvents(this=0x00007f4a8f989c00, waitForEvents=<unavailable>) at IoUringBackend.cpp:1527
    frame facebook#6: 0x0000000009640ac9 ucache`folly::IoUringBackend::eb_event_base_loop(this=0x00007f4a8f989c00, flags=1) at IoUringBackend.cpp:1178
    frame facebook#7: 0x000000001b61547c ucache`folly::EventBase::loopMain(this=0x00007f4a8f97af00, flags=0, options=<unavailable>) at EventBase.cpp:0
    frame facebook#8: 0x000000001bab9a2d ucache`folly::EventBase::loopBody(this=0x00007f4a8f97af00, flags=0, options=(ignoreKeepAlive = false, allowSuspension = false)) at EventBase.cpp:513
    frame facebook#9: 0x000000000a3a6c90 ucache`folly::EventBase::loop(this=<unavailable>) at EventBase.cpp:474
    frame facebook#10: 0x000000000a3a75f7 ucache`folly::EventBase::loopForever(this=<unavailable>) at EventBase.cpp:781
    frame facebook#11: 0x000000000a436907 ucache`folly::IOThreadPoolExecutor::threadRun(this=0x00007f4da3fce1c0, thread=<unavailable>) at IOThreadPoolExecutor.cpp:240
    frame facebook#12: 0x0000000009ed68b8 ucache`void std::__invoke_impl<void, void (folly::ThreadPoolExecutor::*&)(std::shared_ptr<folly::ThreadPoolExecutor::Thread>), folly::ThreadPoolExecutor*&, std::shared_ptr<folly::ThreadPoolExecutor::Thread>&>((null)=<unavailable>, __f=<unavailable>, __t=<unavailable>, __args=<unavailable>) at invoke.h:74
    frame facebook#13: 0x00007f517b6df4e5 libstdc++.so.6`std::execute_native_thread_routine(__p=0x00007f4da01bdbe0) at thread.cc:82:18
    frame facebook#14: 0x00007f517ba9abc9 libc.so.6`start_thread(arg=<unavailable>) at pthread_create.c:434:8
    frame facebook#15: 0x00007f517bb2cf5c libc.so.6`__clone3 at clone3.S:81
```

In frame 2 the offending function is `ReadSqe::callback()`: https://fburl.com/code/4fb46hdq

The problem is that `readCallback_` is null:

```
(lldb) fr v readCallback_
(folly::AsyncReader::ReadCallback *) readCallback_ = nullptr
```

However, `readCallback_` is checked earlier in the the function `ReadSqe::callback()`: https://fburl.com/code/dhnndk29

`AsyncIoUringSocket::readError()` now fails all pending writes and calls `writeErr()`, whose implementation can then close out a socket and call `AsyncIoUringSocket::setReadCB(nullptr)` to change the callback.

Move the call to `AsyncIoUringSocket::readError()` after the call to `readCallback_`.

Reviewed By: dmm-fb

Differential Revision: D60880377

fbshipit-source-id: 42d2f430cd3dc3f4787d0e4c29da7c8ef31ba9a8
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.

5 participants