tanderson69 wrote: ↑
Sat Aug 11, 2018 9:47 pm
The 'BOPCheck.SetParallelMode(true)' call is not needed. That was me probably in a desperate act to speed it up. It also looks like we don't need to copy the shape anymore either (at least for occt 7.3).
IMHO: I would not add a parameter for controlling the multithread of the bopcheck. I would just enable it for the occt versions that have it and let it ride. Can always add it later if it is needed for some strange reason. I wouldn't worry about cpu temps. That is for hardware manufactures, system builders, and overclockers to worry about. As application developers, we should hold the accelerator to the floor and let the machine do what it can do.
This is great! Thanks for working on this. I took a test
from: 443.747654s wall, 443.210000s user + 0.320000s system = 443.530000s CPU (100.0%)
to: 151.161361s wall, 507.330000s user + 39.690000s system = 547.020000s CPU (361.9%)
Thanks for the feedback. I'm not sure I understand those numbers, but the This is great! part, I assume, means you got better performance with it enabled.
You suggest removing the line BOPCheck.SetParallelMode(true)? Any harm in leaving it as is? What is the difference between the two settings (SetParallelMode and SetRunParallel)? Seems to me they should both do the same thing or perhaps work in tandem, but evidently that's not the case. One was listed as a static method while the other was inline. Maybe SetRunParallel is intended as a replacement down the road if it's more thread-safe? I'm trying to adopt the mindset of leaving any existing code alone if it's not necessary to change it.
As for the parameter, I'm of two minds about it. I'm okay with whatever Werner thinks on the subject. One angle to consider is we don't want to go unnecessarily scaring people away from using FreeCAD out of fear of damaging their CPU's. We also don't want to plant seeds of ideas that if something does happen to somebody's CPU maybe it was FreeCAD's fault. From that standpoint the wording should be something along the lines, enabling this option improves performance, but disabling reduces CPU usage, potentially lowering power consumption. (I'm not a lawyer, but I worked in the legal industry for about 5 years...)
I'm also thinking it would be better to make it specific to RunBOPCheck and put it right next to that parameter so when users enable RunBOPCheck they'll see it and be able to decide whether to enable or disable it, too. I don't know whether something similar can be also applied to other boolean operations (hopefully it can improve performance in those areas, too), but if it can and if it is deemed desirable to have a similar parameter for those operations, too, then another parameter can be created for them as the need arises, or perhaps a more general option for all boolean operations.
One argument in favor of the parameter is it makes it easy for users who would like to experiment with having it enabled or disabled to see what, if any, difference it makes. To that end, perhaps enabling the timer message (how long it took to run the test) for all builds, whether debug or not would be warranted?
There is another option, too, the one about ending the check on finding the first faulty that perhaps we should add a parameter for. If it speeds up the check, but at the expense of having fewer results, I think it would be worth doing. Perhaps running in parallel mode will make that option more useful.