storm-devs / storm-engine

Game engine behind Sea Dogs, Pirates of the Caribbean and Age of Pirates games.
https://storm-devs.github.io/storm-engine
GNU General Public License v3.0
847 stars 120 forks source link

[CI] Update Windows CI #476

Closed espkk closed 1 year ago

espkk commented 1 year ago

should be fine now

q4a commented 1 year ago

@espkk can you remove "jobs": 8 ? I think that all has different HW and CI has less then 8 cores.

espkk commented 1 year ago

@espkk can you remove "jobs": 8 ? I think that all has different HW and CI has less then 8 cores.

CI should override thread count with the actual number of cores

q4a commented 1 year ago

Ok. What about people that have 4-6 or 24 cores? I tested on my old Windows PC with 4 cores and this looks wrong: image

espkk commented 1 year ago

Ok. What about people that have 4-6 or 24 cores? I tested on my old Windows PC with 4 cores and this looks wrong: image

It should be possible to override with the -j flag. 8 is an informal default for -j

The problem is that we already have msvc parallel build enabled, so it should be already optimal. While parallel project building increases contention

I'm not sure yet if we can disable it for windows builds only without introducing a new build preset, but since so far it's only used for windows builds, we can just disable it completely

q4a commented 1 year ago

UPD: I removed "jobs": 8 and got better results: image It's still 4-6 threads, but it's not 8 at least.

espkk commented 1 year ago

@q4a, check my previous answer. in worst case it might be cores x 8 on windows

PS don't merge this yet, let me squash