[Fix] Fix eval stage is extremely slow#2409
[Fix] Fix eval stage is extremely slow#2409xming521 wants to merge 1 commit intoopen-compass:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR targets a performance issue where the eval stage is extremely slow due to Python code formatting being applied during config dump operations in the LocalRunner. By setting _format_python_code = False on the task config object before calling dump(), the expensive code formatting step is skipped.
Changes:
- Disables Python code formatting on the task config before dumping it to a temporary file in
LocalRunner._launch()
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| object.__setattr__(task.cfg, "_format_python_code", False) | ||
| task.cfg.dump(param_file) |
There was a problem hiding this comment.
The fix to disable Python code formatting during config dump (line 219) is only applied in the _launch method (the non-debug path of LocalRunner), but the same slow behavior can occur in:
- The debug path of
LocalRunner(local.pyline 110), which also callstask.cfg.dump(param_file)without the fix. - All other runners that call
cfg.dump(param_file)without this optimization:slurm.py(line 99),dlc.py(line 127),rjob.py(line 185),volc.py(line 115),slurm_sequential.py(line 182), andlocal_api.py(line 200).
The fix should be applied consistently to all cfg.dump() call sites across all runners to fully address the performance issue.
| param_file = f'{pwd}/tmp/{uuid_str}_params.py' | ||
|
|
||
| try: | ||
| object.__setattr__(task.cfg, "_format_python_code", False) |
There was a problem hiding this comment.
Using object.__setattr__() to set _format_python_code on the mmengine Config object is a fragile approach that bypasses the class's __setattr__ mechanism and relies on the internal implementation of mmengine's Config. If mmengine changes its internal attribute names or layout, this will silently fail to have any effect (without raising an error), causing the performance issue to reappear undetected.
The rest of the codebase (e.g., opencompass/cli/main.py:301 and opencompass/utils/run.py:98) passes format_python_code=False directly to Config.fromfile(). Since the task config is already a Config object at this point, a more robust alternative is to reload it with the correct flag, or to check if mmengine's Config exposes a public setter for format_python_code and use that instead.
| object.__setattr__(task.cfg, "_format_python_code", False) | |
| # Prefer using a public API to control python code formatting if available | |
| if hasattr(task.cfg, 'format_python_code'): | |
| try: | |
| task.cfg.format_python_code = False | |
| except Exception: | |
| # If setting fails, fall back to default behavior | |
| pass |
Thanks for your contribution and we appreciate it a lot. The following instructions would make your pull request more healthy and more easily get feedback. If you do not understand some items, don't worry, just make the pull request and seek help from maintainers.
Motivation
Please describe the motivation of this PR and the goal you want to achieve through this PR.
Modification
Please briefly describe what modification is made in this PR.
BC-breaking (Optional)
Does the modification introduce changes that break the backward compatibility of the downstream repositories?
If so, please describe how it breaks the compatibility and how the downstream projects should modify their code to keep compatibility with this PR.
Use cases (Optional)
If this PR introduces a new feature, it is better to list some use cases here and update the documentation.
Checklist
Before PR:
After PR: