Thread local context - #359
Conversation
| int m_iHashTableExp; | ||
|
|
||
| public: | ||
| static thread_local std::shared_ptr<COMMON::WorkSpace> m_workspace; |
There was a problem hiding this comment.
I'm not sure it is safe to have workspace handled this way. If we have two objects index A and index B, both can run search on any thread. So if the config of the two index is different, it can cause problems or requires expensive reset each time workspace is used.
What is your opinion on alternative, have workspace passed in as argument and managed by the calling code? Or can we guarantee, even use each other's worspace, index A and B don't have any stability issue or other bug?
There was a problem hiding this comment.
I think the current incompatibilities are
- Maxcheck (deduper config and queue sizes)
- data compression/pagebuffer size
- p_InternalResultNum: Iocp size, posting sizes, etc.
There was a problem hiding this comment.
If we are confident it's safe, let's prove it by unit test compare interleaved search on same thread of A and B to search on different threads. Then we can prevent regression if new feature adds more state to workspace.
There was a problem hiding this comment.
If different indices have different search thread pool, then we can ensure the stability.
| set (NUMA_LIBRARY "") | ||
| set (NUMA_LIBRARY_STATIC "") | ||
| message (STATUS "WARNING: Numa library not found.") | ||
| message (STATUS "Try: 'sudo yum install numactl numactl-devel' (or sudo apt-get install libnuma libnuma-dev)") |
There was a problem hiding this comment.
how would this work for NUMA on Windows? I don't see a reference to a numa nuget pkg or use of the Windows NUMA APIs?
There was a problem hiding this comment.
It's using the simpler processor-group apis, see AsyncFileReader.cpp:138
It might be worthwhile to use the numa apis explicitly if some cpu skus have >64 core in same node
* make it possible to override workspace implementation * bool -> ErrrorCode * SPANN index should allow setting child index workspace * finish replacing by workspace factory * switch to unique_ptr * unresolved external * linux build error * windows build error
This reverts commit 5ffeabe.
This reverts commit 5ffeabe.
This reverts commit 5ffeabe.
This partially reverts commit 5ffeabe.
This partially reverts commit 5ffeabe.
This partially reverts commit 5ffeabe.
This partially reverts commit 5ffeabe.
This partially reverts commit 5ffeabe.
This partially reverts commit 5ffeabe.
This partially reverts commit 5ffeabe.
This partially reverts commit 5ffeabe.
* modify for thread_local context * fix initialization issue * fix ExtraWorkSpace id issue * fix workSpacePool * set thread affinity * add more affinity strategies * fix cmake compiler * fix linux libnuma compile * fix compiling and core bind * fix NumaStrategy and OrderStrategy enum type * remove space * Clear the workspace to ensure the heap size and pagebuffer size * User-overrideable worskpace implementation draft (microsoft#362) * make it possible to override workspace implementation * bool -> ErrrorCode * SPANN index should allow setting child index workspace * finish replacing by workspace factory * switch to unique_ptr * unresolved external * linux build error * windows build error --------- Co-authored-by: cheqi <cheqi@SRGSSD-07> Co-authored-by: Philip Adams <35666630+PhilipBAdams@users.noreply.github.com>
3 participants
No description provided.