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

Refactoring of cpu topology / game fix for The Forest #167

Merged
merged 2 commits into from
Jan 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 3 additions & 5 deletions gamefixes/1659420.py
Original file line number Diff line number Diff line change
@@ -1,14 +1,12 @@
""" Uncharted Collection
""" UNCHARTED: Legacy of Thieves Collection
"""
#pylint: disable=C0103

from protonfixes import util
import multiprocessing


def main():
""" Legacy Collection
""" The game chokes on more than 16 cores
"""

if multiprocessing.cpu_count() > 16:
util.set_environment('WINE_CPU_TOPOLOGY', '16:0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15')
util.set_cpu_topology_limit(16)
5 changes: 1 addition & 4 deletions gamefixes/20920.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,10 @@
#pylint: disable=C0103

from protonfixes import util
import multiprocessing


def main():
""" Witcher 2 chokes on more than 31 cores
"""

if multiprocessing.cpu_count() > 24:
util.set_environment('WINE_CPU_TOPOLOGY', '31:0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26,27,28,29,30')

util.set_cpu_topology_limit(31)
4 changes: 1 addition & 3 deletions gamefixes/220240.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,10 @@
#pylint: disable=C0103

from protonfixes import util
import multiprocessing


def main():
""" FarCry 3 chokes on more than 24 cores
"""

if multiprocessing.cpu_count() > 24:
util.set_environment('WINE_CPU_TOPOLOGY', '25:0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24')
util.set_cpu_topology_limit(24)
14 changes: 14 additions & 0 deletions gamefixes/242760.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
""" Game fix for The Forest
"""

# pylint: disable=C0103

from protonfixes import util


def main():
""" If SMT/HT is enabled, The Forest runs with extremely choppy. Just bad.
We can fix it by setting the topology to the physical cores / core count.
TODO: This fix was not tested with more than 10 physical cores yet.
"""
util.set_cpu_topology_nosmt()
4 changes: 1 addition & 3 deletions gamefixes/298110.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
#pylint: disable=C0103

from protonfixes import util
import multiprocessing


def main():
Expand All @@ -12,5 +11,4 @@ def main():

util.protontricks('d3dcompiler_43')
util.protontricks('d3dcompiler_47')
if multiprocessing.cpu_count() > 24:
util.set_environment('WINE_CPU_TOPOLOGY', '25:0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24')
util.set_cpu_topology_limit(24)
5 changes: 1 addition & 4 deletions gamefixes/35130.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,10 @@
#pylint: disable=C0103

from protonfixes import util
import multiprocessing


def main():
""" LCGoL chokes on more than 12 cores
"""

if multiprocessing.cpu_count() > 24:
util.set_environment('WINE_CPU_TOPOLOGY', '12:0,1,2,3,4,5,6,7,8,9,10,11')

util.set_cpu_topology_limit(12)
16 changes: 4 additions & 12 deletions gamefixes/45750.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,15 +62,7 @@ def main():
except:
log("Unexpected exception when downloading mocked 'xlive.dll'")

# the core fix is only applied if the user has not provided its own topology mapping
if not os.getenv("WINE_CPU_TOPOLOGY"):
try:
cpu_cores = multiprocessing.cpu_count()
except:
cpu_cores = 0
log("Could not retrieve the number of CPU cores")

if cpu_cores > 12:
util.set_environment("WINE_CPU_TOPOLOGY", f"12:{','.join(str(n) for n in range(12))}")


# According to PCGW, no more than 6 physical cores work
# Nevertheless, the game was tested with 12 threads
# TODO: Test the game with SMT disabled / use set_cpu_topology_nosmt()
util.set_cpu_topology_limit(12)
6 changes: 2 additions & 4 deletions gamefixes/55150.py
Original file line number Diff line number Diff line change
@@ -1,15 +1,13 @@
""" Space Marine
""" Warhammer 40,000: Space Marine - Anniversary Edition
"""
#pylint: disable=C0103

from protonfixes import util
import multiprocessing


def main():
""" Space Marine chokes on more than 24 cores
"""

if multiprocessing.cpu_count() > 24:
util.set_environment('WINE_CPU_TOPOLOGY', '25:0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24')
util.set_cpu_topology_limit(24)

77 changes: 77 additions & 0 deletions util.py
Original file line number Diff line number Diff line change
Expand Up @@ -661,3 +661,80 @@ def try_show_gui_error(text):
subprocess.run(["notify-send", "protonfixes", text])
except:
log.info("Failed to show error message with the following text: {}".format(text))

def is_smt_enabled() -> bool:
Copy link

@R1kaB3rN R1kaB3rN Jan 8, 2024

Choose a reason for hiding this comment

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

Nit: Since users should be using set_cpu_topology_nosmt , set_cpu_topology_limit and set_cpu_topology to control the topology, I would prefix the function with an underscore to indicate that they're private and shouldn't be invoked directly

_is_smt_enabled()

Copy link
Author

Choose a reason for hiding this comment

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

I think there are valid reasons to use them in fixes, beyond of the topology functions.
For example setting some cpu variable in an ini file or add an argument.

I know nothing about python, is that really how you mark a function as private?
Crazy. But not that stupid.

Copy link

@R1kaB3rN R1kaB3rN Jan 8, 2024

Choose a reason for hiding this comment

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

Python isn't my strongest language either so I don't know too many of its details/conventions, but I believe the runtime wouldn't enforce the function as "private" in the classical OOP sense. As a result, you'd still be able to access them externally without a runtime error

Instead, the underscore would really serve as a friendly indicator that it's intended for internal use. At least, that's what I had assumed as the convention in this codebase. For example, _killhanging() in util.py

Copy link

@R1kaB3rN R1kaB3rN Jan 8, 2024

Choose a reason for hiding this comment

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

I think there are valid reasons to use them in fixes, beyond of the topology functions.

There are valid reasons. It's just I can't foresee how useful those functions would likely be when used externally from a game fix file outside the topology context...

Though, it's just a nit and don't really mind

""" Returns whether SMT is enabled.
If the check has failed, False is returned.
"""
try:
with open('/sys/devices/system/cpu/smt/active') as smt_file:
return smt_file.read().strip() == "1"
except PermissionError:
log.warn('No permission to read SMT status')
except OSError as e:
log.warn(f'SMT status not supported by the kernel (errno: {e.errno})')
return False

def get_cpu_count() -> int:
Copy link

Choose a reason for hiding this comment

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

_get_cpu_count()

""" Returns the cpu core count, provided by the OS.
If the request failed, 0 is returned.
"""
cpu_cores = os.cpu_count()
if not cpu_cores or cpu_cores <= 0:
log.warn('Can not read count of logical cpu cores')
return 0
return cpu_cores

def set_cpu_topology(core_count: int, ignore_user_setting: bool = False) -> bool:
""" This sets the cpu topology to a fixed core count.
By default, a user provided topology is prioritized.
You can override this behavior by setting `ignore_user_setting`.
"""

# Don't override the user's settings (except, if we override it)
user_topo = os.getenv('WINE_CPU_TOPOLOGY')
if user_topo and not ignore_user_setting:
log.info(f'Using WINE_CPU_TOPOLOGY set by the user: {user_topo}')
return False

# Sanity check
if not core_count or core_count <= 0:
log.warn('Only positive core_counts can be used to set cpu topology')
return False

# Format (example, 4 cores): 4:0,1,2,3
cpu_topology = f'{core_count}:{",".join(map(str, range(core_count)))}'
set_environment('WINE_CPU_TOPOLOGY', cpu_topology)
log.info(f'Using WINE_CPU_TOPOLOGY: {cpu_topology}')
return True


def set_cpu_topology_nosmt(core_limit: int = 0, ignore_user_setting: bool = False, threads_per_core: int = 2) -> bool:
""" This sets the cpu topology to the count of physical cores.
If SMT is enabled, eg. a 4c8t cpu is limited to 4 logical cores.
You can limit the core count to the `core_limit` argument.
"""

# Check first, if SMT is enabled
if is_smt_enabled() is False:
log.info('SMT is not active, skipping fix')
return False

# Currently (2024) SMT allows 2 threads per core, this might change in the future
cpu_cores = get_cpu_count() // threads_per_core # Apply divider
cpu_cores = max(cpu_cores, min(cpu_cores, core_limit)) # Apply limit
return set_cpu_topology(cpu_cores, ignore_user_setting)

def set_cpu_topology_limit(core_limit: int, ignore_user_setting: bool = False) -> bool:
""" This sets the cpu topology to a limited number of logical cores.
A limit that exceeds the available cores, will be ignored.
"""

cpu_cores = get_cpu_count()
if core_limit >= cpu_cores:
log.info(f'The count of logical cores ({cpu_cores}) is lower than '
f'or equal to the set limit ({core_limit}), skipping fix')
return False

# Apply the limit
return set_cpu_topology(core_limit, ignore_user_setting)