forked from simons-public/protonfixes
-
Notifications
You must be signed in to change notification settings - Fork 72
Refactoring of cpu topology / game fix for The Forest #167
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
Merged
GloriousEggroll
merged 2 commits into
GloriousEggroll:master
from
Root-Core:cpu_topology
Jan 8, 2024
Merged
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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() |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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) | ||
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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: | ||
""" 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: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
""" 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) |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
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
andset_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 directlyThere was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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