Skip to content

refactor: parallelize unsupported-order detection and defer filtering#4309

Open
metalurgical wants to merge 3 commits intocowprotocol:mainfrom
metalurgical:issue_3516
Open

refactor: parallelize unsupported-order detection and defer filtering#4309
metalurgical wants to merge 3 commits intocowprotocol:mainfrom
metalurgical:issue_3516

Conversation

@metalurgical
Copy link
Copy Markdown

@metalurgical metalurgical commented Apr 5, 2026

Description

Parallelize unsupported-order detection and defer filtering

Changes

  • Switch risk detection to return UIDs,
  • run it concurrently with preprocessing,
  • apply filtering after updating orders.
    - 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 #4308 Split from PR

@metalurgical metalurgical requested a review from a team as a code owner April 5, 2026 20:23
@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 5, 2026

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@metalurgical
Copy link
Copy Markdown
Author

I have read the CLA Document and I hereby sign the CLA

@metalurgical
Copy link
Copy Markdown
Author

recheck

github-actions bot added a commit that referenced this pull request Apr 5, 2026
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@jmg-duarte
Copy link
Copy Markdown
Contributor

I believe this change is built on top of your other PR, please separate them for easier review

Copy link
Copy Markdown
Contributor

@jmg-duarte jmg-duarte left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@metalurgical
Copy link
Copy Markdown
Author

@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.

@metalurgical metalurgical changed the title refactor: parallelize unsupported-order detection and refactor sorting to avoid order cloning refactor: parallelize unsupported-order detection and defer filtering Apr 6, 2026
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.
@jmg-duarte
Copy link
Copy Markdown
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

chore(driver): parallelize without_unsupported_orders execution

2 participants