Skip to content

[Fix] Fix eval stage is extremely slow#2409

Open
xming521 wants to merge 1 commit intoopen-compass:mainfrom
xming521:main
Open

[Fix] Fix eval stage is extremely slow#2409
xming521 wants to merge 1 commit intoopen-compass:mainfrom
xming521:main

Conversation

@xming521
Copy link

@xming521 xming521 commented Mar 5, 2026

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:

  • Pre-commit or other linting tools are used to fix the potential lint issues.
  • Bug fixes are fully covered by unit tests, the case that causes the bug should be added in the unit tests.
  • The modification is covered by complete unit tests. If not, please add more unit test to ensure the correctness.
  • The documentation has been modified accordingly, like docstring or example tutorials.

After PR:

  • If the modification has potential influence on downstream or other related projects, this PR should be tested with those projects.
  • CLA has been signed and all committers have signed the CLA in this PR.

Copilot AI review requested due to automatic review settings March 5, 2026 11:51
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

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.

Comment on lines +219 to 220
object.__setattr__(task.cfg, "_format_python_code", False)
task.cfg.dump(param_file)
Copy link

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

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

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:

  1. The debug path of LocalRunner (local.py line 110), which also calls task.cfg.dump(param_file) without the fix.
  2. 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), and local_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.

Copilot uses AI. Check for mistakes.
param_file = f'{pwd}/tmp/{uuid_str}_params.py'

try:
object.__setattr__(task.cfg, "_format_python_code", False)
Copy link

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

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

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.

Suggested change
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

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants