-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
PoissonRecon on Windows fails on certain machines with "Failed to open file:" #1307
Comments
Another user reported a memory error after replacing the binary with the official PoissonRecon.exe release:
Probably due to the number of cores ? |
Piero, this may be stupid (and I can't repro locally since old machine), but what if the issue is related to like... number size/allocation overflow. Something with number of threads taking two integers to report (10 threads or more) instead of one integer (9 threads or less). If that's correct, someone locking their max-concurrency to 9 should be 100% fine all day even on the newer CPUs. |
I've been asking a few people to try this (limit max-concurrency), but haven't heard consistent reports back on whether it works as a workaround. It might. |
Would it make sense to have a "min-concurrency" flag for testing so I could see if this behavior can be reproduced on older "stable" CPUs just by pushing concurrency above 9? |
For testing purposes, I would probably just patch Line 161 in 901cd8f
|
Seems to be working just fine still with threads modified to be 16 (CPU is 4 physical, 8 logical). I got nothing... AVX/SSE thing? Hardware specter mitigation issue? |
This, along with OpenDroneMap/NodeODM#158 are both puzzling; I'm not really sure of the root cause. They both seem filesystem related. |
I've been able to reproduce this on my machine with:
So I bumped the number of threads. This does look like a race condition of some sort. |
Even the official binaries crash once in a while, yet with other errors:
|
Workaround in place as part of #1308. |
Machines with lots of cores seem more affected than others ?
The text was updated successfully, but these errors were encountered: