refactor: parallelize unsupported-order detection and defer filtering#4309
refactor: parallelize unsupported-order detection and defer filtering#4309metalurgical wants to merge 3 commits intocowprotocol:mainfrom
Conversation
|
All contributors have signed the CLA ✍️ ✅ |
|
I have read the CLA Document and I hereby sign the CLA |
|
recheck |
There was a problem hiding this comment.
Code Review
This pull request enhances cross-platform support and optimizes the auction preprocessing pipeline. Significant changes include making memory allocators and system signals platform-specific, updating database configuration for Windows compatibility, and refactoring order sorting and filtering to minimize cloning and improve parallelism. No critical issues found.
|
I believe this change is built on top of your other PR, please separate them for easier review |
jmg-duarte
left a comment
There was a problem hiding this comment.
The typo fixes and import shortening changes are appreciated overall but in this PR they bring in noise to a review that must be done carefully
The parallelism change is also semi-buried in the sorting order refactor
The sorting order refactor is non-trivial and it lacks safety justifications as to why it works. It could also be abstracted/simplified since it doesn't matter if a vector contains T, Z, or A if we're ignoring the type and using another slice as "sorting reference"
Finally, I don't understand why there's so much documentation being removed
|
@jmg-duarte You raised valid points. Have simplified it to minimal correction for the issue, restored the documentation comments and removed the permutation based sorting. |
Move unsupported-order detection to a parallel, read-only stage in the auction preprocessing pipeline. Detection now operates on a snapshot of orders and returns UIDs to discard, rather than mutating the auction early.
|
Hey @metalurgical thanks for the changes! It looks much better now, I haven't had a chance of giving them a through review yet; if you don't mind resolving the conflicts, that would be great. Full disclosure: The PR will need to go through internal testing to ensure there's no performance regressions, I'll take care of that and either bring comments or iterate directly on it myself (depends on outcomes); however, it might take a bit to do so due to this not being a top priority now. Once more, thanks for your contribution |
Description
Parallelize unsupported-order detection and defer filtering
Changes
- Replace in-place sorting with permutation-based ordering to avoid cloning orders.How to test
As normal.
Related Issues
Fixes #3516
Note
Built on top of #4308Split from PR