elucideye / drishti

Real time eye tracking for embedded and mobile devices.
BSD 3-Clause "New" or "Revised" License
391 stars 82 forks source link

thread-pool-cpp: thread_local issues #195

Open headupinclouds opened 7 years ago

headupinclouds commented 7 years ago

The ios toolchains builds are aborting with a linker error that seems to be related to use of __thread or thread_local use in thread-pool-cpp via a shared library.

https://github.com/hunter-packages/thread-pool-cpp/blob/hunter/thread_pool/worker.hpp#L4-L12

// http://stackoverflow.com/a/25393790/5724090
// Static/global variable exists in a per-thread context (thread local storage).
#if defined (__GNUC__)
    #define ATTRIBUTE_TLS __thread
#elif defined (_MSC_VER)
    #define ATTRIBUTE_TLS __declspec(thread)
#else // !__GNUC__ && !_MSC_VER
    #error "Define a thread local storage qualifier for your compiler/platform!"
#endif

Note: Recent Xcode versions should support C++11 thread_local types, and this could be added to thread-pool-cpp, although this also triggers the same crash.

The ld crash seems to be related to assignment to the the thread_local type in this routine:

template <size_t STORAGE_SIZE>
inline void Worker<STORAGE_SIZE>::threadFunc(size_t id, Worker *steal_donor)
{
    *detail::thread_id() = id; // <= remove this to avoid crash (obviously required)
    // ...
}
headupinclouds commented 7 years ago
 /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ld -r -arch arm64 -syslibroot /Applications/Xcode.app/Contents/Developer/Platforms/iPhoneOS.platform/Developer/SDKs/iPhoneOS9.3.sdk /Users/travis/build/elucideye/drishti/_builds/ios-nocodesign-9-3-device/src/lib/drishti/drishtisdk.build/Release-iphoneos/drishti.build/Objects-normal/arm64/Eye.o /Users/travis/build/elucideye/drishti/_builds/ios-nocodesign-9-3-device/src/lib/drishti/drishtisdk.build/Release-iphoneos/drishti.build/Objects-normal/arm64/EyeDetector.o /Users/travis/build/elucideye/drishti/_builds/ios-nocodesign-9-3-device/src/lib/drishti/drishtisdk.build/Release-iphoneos/drishti.build/Objects-normal/arm64/EyeSegmenter.o /Users/travis/build/elucideye/drishti/_builds/ios-nocodesign-9-3-device/src/lib/drishti/drishtisdk.build/Release-iphoneos/drishti.build/Objects-normal/arm64/EyeSegmenterImpl.o /Users/travis/build/elucideye/drishti/_builds/ios-nocodesign-9-3-device/src/lib/drishti/drishtisdk.build/Release-iphoneos/drishti.build/Objects-normal/arm64/FaceTracker.o /Users/travis/build/elucideye/drishti/_builds/ios-nocodesign-9-3-device/src/lib/drishti/drishtisdk.build/Release-iphoneos/drishti.build/Objects-normal/arm64/Image.o /Users/travis/build/elucideye/drishti/_builds/ios-nocodesign-9-3-device/src/lib/drishti/drishtisdk.build/Release-iphoneos/drishti.build/Objects-normal/arm64/Manager.o /Users/travis/build/elucideye/drishti/_builds/ios-nocodesign-9-3-device/src/lib/drishti/drishtisdk.build/Release-iphoneos/drishti.build/Objects-normal/arm64/drishti_sdk.o -o /Users/travis/build/elucideye/drishti/_builds/ios-nocodesign-9-3-device/src/lib/drishti/drishtisdk.build/Release-iphoneos/drishti.build/Objects-normal/libdrishti.dylib-arm64-master.o
0  0x10ff6599e  __assert_rtn + 144
1  0x10ffbc9e6  ld::tool::SectionRelocationsAtom<arm64>::encodeSectionReloc(ld::Internal::FinalSection*, ld::tool::SectionRelocationsAtom<arm64>::Entry const&, std::__1::vector<macho_relocation_info<Pointer64<LittleEndian> >, std::__1::allocator<macho_relocation_info<Pointer64<LittleEndian> > > >&) + 1618
2  0x10ffe730a  ld::tool::SectionRelocationsAtom<arm64>::encode() + 62
3  0x10ffc274d  ld::tool::OutputFile::updateLINKEDITAddresses(ld::Internal&) + 413
4  0x10ffbce69  ld::tool::OutputFile::write(ld::Internal&) + 167
5  0x10ff6691a  main + 1311
6  0x7fff882ee5ad  start + 1
A linker snapshot was created at:
  /tmp/libdrishti.dylib-arm64-master.o-2016-11-03-174823.ld-snapshot
ld: Assertion failed: (0 && "need to handle arm64 -r reloc"), function encodeSectionReloc, file /Library/Caches/com.apple.xbs/Sources/ld64/ld64-264.3.102/src/ld/LinkEditClassic.hpp, line 1907.
Command /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ld failed with exit code 1
headupinclouds commented 7 years ago

Possible thread_local workarounds?

https://github.com/readium/readium-sdk/blob/b36287954790201368922649479f6fccabf89d6d/ePub3/utilities/future.cpp#L130-L166

This could work as a temporary fix, but it is likely to be very slow (although not a bottleneck for current use cases):

    size_t * thread_id()
    {
#if USE_THIS
        static ATTRIBUTE_TLS size_t tss_id = -1u;
        //static thread_local size_t tss_id __attribute__ ((tls_model ("local-exec"))) = -1u;        
        return &tss_id;        
#else
        // Slow workaround
        static pthread_key_t __key;
        static std::once_flag __once;
        std::call_once(__once, [&](){ pthread_key_create(&__key, &__KillSizePtr); });

        void* __p = pthread_getspecific(__key);
        if (__p == nullptr)
        {
            __p = new size_t();
            pthread_setspecific(__key, __p);
        }

        size_t * __pv = static_cast<size_t*>(__p);
        return __pv;
#endif        

    }
headupinclouds commented 7 years ago

Interesting read: http://david-grs.github.io/tls_performance_overhead_cost_linux/

TL;DR: The inline access of thread_local storage id in thread-pool-cpp may be sufficient to avoid overhead in shared libs.

headupinclouds commented 7 years ago

I've isolated the problem to the following setting: XCODE_ATTRIBUTE_GENERATE_MASTER_OBJECT_FILE "YES". It seems the other strip commands are okay to run.

See comments below:

  set_target_properties(${drishti_library}
    PROPERTIES
    XCODE_ATTRIBUTE_COPY_PHASE_STRIP "YES"
    XCODE_ATTRIBUTE_STRIP_INSTALLED_PRODUCT "YES"
    XCODE_ATTRIBUTE_STRIP_STYLE "non-global"
    XCODE_ATTRIBUTE_STRIPFLAGS "-x -u -r"
    XCODE_ATTRIBUTE_DEAD_CODE_STRIPPING "YES"
    XCODE_ATTRIBUTE_DEPLOYMENT_POSTPROCESSING "YES"

    # Note: Using this option in combination with thread_local and shared libraries crashes
    # the linker step with the following error:
    #
    # ld: Assertion failed: (0 && "need to handle arm64 -r reloc"), function encodeSectionReloc,
    # file /Library/Caches/com.apple.xbs/Sources/ld64/ld64-264.3.102/src/ld/LinkEditClassic.hpp, line 1907.
    #
    # This should can be enabled optionally for modules that do not require thread_local.

    # XCODE_ATTRIBUTE_GENERATE_MASTER_OBJECT_FILE "YES" # "Perform Single-Object Prelink"
    )
headupinclouds commented 7 years ago

Here is a minimal project to reproduce the issue: https://github.com/headupinclouds/test_thread_local