feat: thread pool #40
Conversation
jrainville
left a comment
jrainville
left a comment
There was a problem hiding this comment.
Good job on finding the issue and fixing it
| ) | ||
|
|
||
| var | ||
| ctxPool: seq[ptr SdsContext] |
There was a problem hiding this comment.
seq cannot be used cross-thread until we migrate to the orc memory manager meaning that this list needs to be managed manually - the absolutely easiest way to do that is to use an array + counter and limit the number of sds contexts that can be created.
| ) | ||
|
|
||
| var | ||
| ctxPool: seq[ptr SdsContext] |
There was a problem hiding this comment.
| ctxPool: seq[ptr SdsContext] | |
| ctxPool: array[32, ptr SdsContext] | |
| ctxPos: int |
something like this - assuming you don't want more than 32 concurrent contexts
There was a problem hiding this comment.
I think it's okay for now to allow as much as needed.
There was a problem hiding this comment.
I don't know how many is needed. And eventually we'll need a proper fix anyway
There was a problem hiding this comment.
no, I mean using a seq here is undefined behavior - if it grows more than once, it'll crash - seqs are tied to a particular thread, arrays are not - for a cross-thread seq, one needs to use createShared and a bit of extra work (similar to sharedseq in waku / ffi)
There was a problem hiding this comment.
We tested this to work for now in Go tests and Status App.
We agreed with @Ivansete-status to sort this out in next releases.
igor-sirotin
force-pushed
the
fix/thread-pool
branch
from
d026a2c to
2be27d0
Compare
Fixes status-im/status-go#7216