[23.1] Backport tool mem fixes #16601
Merged
Conversation
This file contains 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 is in the docutils source code:
```
if level >= self.halt_level:
raise SystemMessage(msg, level)
```
and it's not being caught. Might be new in new docutils?
This needs a whole bunch of refactoring.
Comparing each element of the static options with each incoming element
is very inefficient. Since dictionaries are ordered this can be
simplified to a simple key value insert.
Prior to this change:
```
parse_static_options (/Users/mvandenb/src/galaxy/lib/galaxy/tool_util/parser/xml.py:1207)
function called 7183 times
313317 function calls in 11.318 seconds
Ordered by: cumulative time, internal time, call count
ncalls tottime percall cumtime percall filename:lineno(function)
7183 11.281 0.002 11.318 0.002 xml.py:1207(parse_static_options)
99675 0.029 0.000 0.033 0.000 __init__.py:1004(string_as_bool)
99675 0.004 0.000 0.004 0.000 {method 'lower' of 'str' objects}
99601 0.004 0.000 0.004 0.000 {method 'append' of 'list' objects}
7183 0.000 0.000 0.000 0.000 {method 'disable' of '_lsprof.Profiler' objects}
0 0.000 0.000 profile:0(profiler)
```
after:
```
*** PROFILER RESULTS ***
parse_static_options (/Users/mvandenb/src/galaxy/lib/galaxy/tool_util/parser/xml.py:1207)
function called 7183 times
220899 function calls in 0.260 seconds
Ordered by: cumulative time, internal time, call count
ncalls tottime percall cumtime percall filename:lineno(function)
7183 0.230 0.000 0.259 0.000 xml.py:1207(parse_static_options)
99675 0.026 0.000 0.029 0.000 __init__.py:1004(string_as_bool)
99675 0.004 0.000 0.004 0.000 {method 'lower' of 'str' objects}
7183 0.000 0.000 0.000 0.000 {method 'values' of 'dict' objects}
7183 0.000 0.000 0.000 0.000 {method 'disable' of '_lsprof.Profiler' objects}
0 0.000 0.000 profile:0(profiler)
```
This is most due to openms_idfilter, which has 2928 static options.
functools.cache is new in python 3.9
For details see benoitc/gunicorn#1640 and https://instagram-engineering.com/copy-on-write-friendly-python-garbage-collection-ad6ed5233ddf I think this is the most subtle to test change. I believe this is working. I started a gunicorn instance with 4 workers: ``` GALAXY_CONFIG_FILE="config/galaxy.yml" gunicorn 'galaxy.webapps.galaxy.fast_factory:factory()' -k galaxy.webapps.galaxy.workers.Worker --pythonpath lib --bind=localhost:8080 --config lib/galaxy/web_stack/gunicorn_config.py --preload -w 4 ``` Then i use the following script against that instance ``` import threading import requests def req(): for i in range(10000): requests.get('http://localhost:8080/history/current_history_json') for i in range(10): threading.Thread(target=req).start() ``` I see that the memory consumption increases much more *during* requests without this commit. It eventually decreases again, but I think not to the same baseline level (hard to tell without more elaborate testing). I attribute the higher memory load during requests to the fact that the garbage collection requiring to inspect more objects, taking more time to run and therefor not running as fast? I'm really not sure, I think we should just roll this out and see, it should be fairly obvious from the grafana dashboards.
mvdbeek
force-pushed
the
backport_tool_mem_fixes
branch
from
e74727e
to
e0ba54e
Compare
github-actions
bot
added
area/workflows
area/admin
area/tool-framework
area/util
labels
bgruening
reviewed
Aug 26, 2023
| # freeze objects after preloading app | ||
| if is_preload_app(): | ||
| gc.freeze() | ||
| print("Objects frozen in perm gen: ", gc.get_freeze_count()) |
There was a problem hiding this comment.
No, I think that's good to have
bgruening
approved these changes
Aug 26, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Backport of #16536
It's not entirely a bugfix, but there are commits in there that are definitely fixing bugs, and I plan to test this out on usegalaxy.org
How to test the changes?
(Select all options that apply)
License