Skip to content
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

Fix/parallel repair #130

Conversation

rahulghangas
Copy link
Contributor

solveCrosswordRow sets cells for the crossword which are accessed in solveCrosswordColumn, which means that both being run in parallel has a race condition.

This PR resolves this by running all solveCrosswordRow in parallel and waiting for all these calls to finish before executing all solveCrosswordColumn calls in parallel

@rahulghangas
Copy link
Contributor Author

@musalbas replying to questions from #129

This seems to imply the reason why parallelisation_crosswordloop didn't work was not because of the decoder, but because of rsmt2d. In that case how come it worked with infectious but not leopard or klauspost?

Infectious was failing on my machine, although sporadically. I think it was just the race condition at play.

Blocking column repair until row repair is complete means that we aren't maximizing the parallelisation that we can do. Is there another way to fix this, eg add mutexes for setting cells?

I don't think so, the race condition is between reading and writing of cells. For eg. solveCrosswordRow is setting cells here while the same cell is being read by solveCrosswordCol here. iiuc, the cell must set with the rebuilt shares before being read

@musalbas
Copy link
Member

PR should be into parallelisation_crosswordloop rather than parallelisation_klauspost as the latter is just a testing branch with leopard replaced with klauspost

@rahulghangas rahulghangas deleted the fix/parallel-repair branch September 28, 2022 13:19
@rahulghangas rahulghangas mentioned this pull request Sep 28, 2022
1 task
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.

2 participants