-
Notifications
You must be signed in to change notification settings - Fork 298
fixes for parallel reconstruction #7758
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
base: unstable
Are you sure you want to change the base?
Conversation
7bf20ac to
2b0a983
Compare
|
please re-run macos job I had to kill it to clean up |
| var spawned = 0 | ||
|
|
||
| # Choose a sane limit for concurrent tasks to reduce peak memory/alloc pressure. | ||
| let maxInFlight = if blobCount < 9: blobCount else: 9 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
min(blobCount, 9)
|
|
||
| # ---- Spawn phase with time limit ---- | ||
| proc freePendingPtrPair(idxPtr: ptr CellIndex, cellsPtr: ptr Cell) = | ||
| if idxPtr != nil: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
free(NULL)/free(0) is fine and doesn't have to be guarded against
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://www.open-std.org/JTC1/SC22/wg14/www/docs/n1124.pdf
7.20.3.2 The free function
...
The free function causes the space pointed to by ptr to be deallocated, that is, made available for further allocation. If ptr is a null pointer, no action occurs
| # Allocate unmanaged C buffers and copy data into them | ||
| let | ||
| idxBytes = csize_t(columnCount) * csize_t(sizeof(CellIndex)) | ||
| cellsBytes = csize_t(columnCount) * csize_t(sizeof(Cell)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is what calloc does. It also accounts for potential alignment issues (i.e. due to alignment requirements, the size isn't just n*sizeof(y))
| idxPtr = cast[ptr CellIndex](c_malloc(idxBytes)) | ||
| if idxPtr == nil: | ||
| drainPending(0) | ||
| return err("Out of memory") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be good to have this Out of memory message be slightly more specific about its context and be easily distinguishable should it ever appear.
| if cellsPtr == nil: | ||
| c_free(idxPtr) | ||
| drainPending(0) | ||
| return err("Out of memory") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be good to have this Out of memory message be slightly more specific about its context and be easily distinguishable should it ever appear.
| localIndices[j] = idxArr[j] | ||
| localCells[j] = cellsArr[j] | ||
| # use the task wrapper which maps string errors to void | ||
| return recoverCellsAndKzgProofsTask(localIndices, localCells) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't need return here
No description provided.