Skip to content

Conversation

@MengjinYan
Copy link
Contributor

@MengjinYan MengjinYan commented Oct 14, 2025

Description

Recently, when we ran performance tests with task event generation turned on. We saw some performance regression when the workloads ran on very small CPU machines. With further investigation, the overhead mainly comes from the name format convention when converting the proto message to JSON format payload in the aggregator agent.

This PR adds an env var for the config to control the name conversion behavior and update the corresponding tests.

Also note that, eventually we are planning to remove this config turn off the field name conversion by default after migrated all the current event usage.

Related issues

N/A

Types of change

  • Bug fix 🐛
  • New feature ✨
  • Enhancement 🚀
  • Code refactoring 🔧
  • Documentation update 📖
  • Chore 🧹
  • Style 🎨

Checklist

Does this PR introduce breaking changes?

  • Yes ⚠️
  • No

Testing:

  • Added/updated tests for my changes
  • Tested the changes manually
  • This PR is not tested ❌ (please explain why)

Code Quality:

  • Signed off every commit (git commit -s)
  • Ran pre-commit hooks (setup guide)

Documentation:

  • Updated documentation (if applicable) (contribution guide)
  • Added new APIs to doc/source/ (if applicable)

Additional context

@MengjinYan MengjinYan added the go add ONLY when ready to merge, run all tests label Oct 17, 2025
@MengjinYan MengjinYan changed the title [Draft][Core] Turn off JSON Property Name Conversion [Core] Make Preserving Proto Naming During Event JSON Conversion Oct 17, 2025
@MengjinYan MengjinYan changed the title [Core] Make Preserving Proto Naming During Event JSON Conversion [Core] Make Preserving Proto Naming During Event JSON Conversion Configurable Oct 17, 2025
@MengjinYan MengjinYan marked this pull request as ready for review October 20, 2025 05:21
@MengjinYan MengjinYan requested a review from a team as a code owner October 20, 2025 05:21
@ray-gardener ray-gardener bot added the core Issues that should be addressed in Ray Core label Oct 20, 2025
@edoakes
Copy link
Collaborator

edoakes commented Oct 20, 2025

Is making it configurable a temporary measure or long term? I wouldn't keep both in perpetuity.

@MengjinYan
Copy link
Contributor Author

Is making it configurable a temporary measure or long term? I wouldn't keep both in perpetuity.

It will be temporary. I'm planning to remove this config to turn off the field name conversion by default after migrated all the current event usage.

@edoakes edoakes merged commit 9990ebd into ray-project:master Oct 20, 2025
7 checks passed
xinyuangui2 pushed a commit to xinyuangui2/ray that referenced this pull request Oct 22, 2025
…igurable (ray-project#57705)

Recently, when we ran performance tests with task event generation
turned on. We saw some performance regression when the workloads ran on
very small CPU machines. With further investigation, the overhead mainly
comes from the name format convention when converting the proto message
to JSON format payload in the aggregator agent.

This PR adds an env var for the config to control the name conversion
behavior and update the corresponding tests.

Also note that, eventually we are planning to remove this config turn
off the field name conversion by default after migrated all the current
event usage.

---------

Signed-off-by: Mengjin Yan <mengjinyan3@gmail.com>
Signed-off-by: xgui <xgui@anyscale.com>
elliot-barn pushed a commit that referenced this pull request Oct 23, 2025
…igurable (#57705)

Recently, when we ran performance tests with task event generation
turned on. We saw some performance regression when the workloads ran on
very small CPU machines. With further investigation, the overhead mainly
comes from the name format convention when converting the proto message
to JSON format payload in the aggregator agent.

This PR adds an env var for the config to control the name conversion
behavior and update the corresponding tests.

Also note that, eventually we are planning to remove this config turn
off the field name conversion by default after migrated all the current
event usage.

---------

Signed-off-by: Mengjin Yan <mengjinyan3@gmail.com>
Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com>
landscapepainter pushed a commit to landscapepainter/ray that referenced this pull request Nov 17, 2025
…igurable (ray-project#57705)

Recently, when we ran performance tests with task event generation
turned on. We saw some performance regression when the workloads ran on
very small CPU machines. With further investigation, the overhead mainly
comes from the name format convention when converting the proto message
to JSON format payload in the aggregator agent.

This PR adds an env var for the config to control the name conversion
behavior and update the corresponding tests.

Also note that, eventually we are planning to remove this config turn
off the field name conversion by default after migrated all the current
event usage.

---------

Signed-off-by: Mengjin Yan <mengjinyan3@gmail.com>
Aydin-ab pushed a commit to Aydin-ab/ray-aydin that referenced this pull request Nov 19, 2025
…igurable (ray-project#57705)

Recently, when we ran performance tests with task event generation
turned on. We saw some performance regression when the workloads ran on
very small CPU machines. With further investigation, the overhead mainly
comes from the name format convention when converting the proto message
to JSON format payload in the aggregator agent.

This PR adds an env var for the config to control the name conversion
behavior and update the corresponding tests.

Also note that, eventually we are planning to remove this config turn
off the field name conversion by default after migrated all the current
event usage.

---------

Signed-off-by: Mengjin Yan <mengjinyan3@gmail.com>
Signed-off-by: Aydin Abiar <aydin@anyscale.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core Issues that should be addressed in Ray Core go add ONLY when ready to merge, run all tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants