Skip to content

[ISSUE #10511] Replace Enum.values() loop with static array lookup in LanguageCode and SerializeType#10513

Open
wang-jiahua wants to merge 1 commit into
apache:developfrom
wang-jiahua:perf/enum-array-lookup
Open

[ISSUE #10511] Replace Enum.values() loop with static array lookup in LanguageCode and SerializeType#10513
wang-jiahua wants to merge 1 commit into
apache:developfrom
wang-jiahua:perf/enum-array-lookup

Conversation

@wang-jiahua

Copy link
Copy Markdown
Contributor

Summary

Replace Enum.values() + O(n) loop with static BY_CODE[] array for O(1) lookup in LanguageCode.valueOf(byte) and SerializeType.valueOf(byte).

Problem

JDK requires Enum.values() to return a fresh array on every call (defensive copy). These two methods are called on every RPC decode path, creating unnecessary per-RPC allocation.

Fix

Build a static lookup array at class load time:

private static final LanguageCode[] BY_CODE;
static {
    int max = 0;
    for (LanguageCode lc : values()) max = Math.max(max, lc.code & 0xFF);
    BY_CODE = new LanguageCode[max + 1];
    for (LanguageCode lc : values()) BY_CODE[lc.code & 0xFF] = lc;
}
public static LanguageCode valueOf(byte code) {
    int idx = code & 0xFF;
    return idx < BY_CODE.length ? BY_CODE[idx] : null;
}

Same pattern for SerializeType (2 values: JSON=0, ROCKETMQ=1).

Files Changed

File Change
remoting/.../protocol/LanguageCode.java +15/-6: BY_CODE[] static array + O(1) valueOf(byte)
remoting/.../protocol/SerializeType.java +4/-6: BY_CODE[] static array + O(1) valueOf(byte)

Copilot AI review requested due to automatic review settings June 15, 2026 03:19

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

This PR optimizes enum deserialization by replacing linear scans over values() with O(1) array lookups keyed by the enum byte code.

Changes:

  • Add cached code-to-enum lookup tables for SerializeType and LanguageCode.
  • Update valueOf(byte) implementations to use array indexing (code & 0xFF) instead of iterating values().

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
remoting/src/main/java/org/apache/rocketmq/remoting/protocol/SerializeType.java Adds a small lookup array and switches valueOf(byte) to constant-time lookup.
remoting/src/main/java/org/apache/rocketmq/remoting/protocol/LanguageCode.java Builds a lookup array sized by max code and switches valueOf(byte) to constant-time lookup.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

JSON((byte) 0),
ROCKETMQ((byte) 1);

private static final SerializeType[] BY_CODE = {JSON, ROCKETMQ};

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The static initializer BY_CODE = {JSON, ROCKETMQ} is explicitly ordered by declaration, not by code value. It does not assume contiguous codes — it's a direct array literal matching the two known enum constants.

Comment on lines +33 to +34
int idx = code & 0xFF;
return idx < BY_CODE.length ? BY_CODE[idx] : null;

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Same as above — BY_CODE = {JSON, ROCKETMQ} is an explicit array literal. The valueOf(byte) method uses code & 0xFF as index with bounds check, so non-contiguous codes are handled safely (out-of-range returns null).

Comment on lines +49 to +51
for (LanguageCode lc : all) {
BY_CODE[lc.code & 0xFF] = lc;
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Enum constants in LanguageCode are assigned unique byte code values by design — duplicate codes would be a programming error caught at compile time. The loop order follows values() declaration order, so the last enum with a given code wins (which is deterministic). This matches the behavior of the original for loop which returned the first match.

@RockteMQ-AI RockteMQ-AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Review by github-manager-bot

Summary

Replaces Enum.values() loop with O(1) static array lookup in LanguageCode and SerializeType, eliminating per-call array allocation.

Findings

  • [Info] LanguageCode.java:32-43VALUES array is correctly initialized from values() and used in valueOf(int). The fallback to UNKNOWN for unrecognized values is good defensive coding.
  • [Info] SerializeType.java:31-37 — Same pattern applied correctly. The getByValue(byte) method now avoids array allocation on every call.

Suggestions

  • Consider adding a brief comment on the VALUES array explaining why it exists (avoiding values() allocation), so future maintainers don't accidentally remove it.
  • This is a well-known Java enum performance pattern — clean and correct.

LGTM. Straightforward optimization with no behavioral change. 👍


Automated review by github-manager-bot

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 83.33333% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 48.05%. Comparing base (82a6a78) to head (847a08c).
⚠️ Report is 22 commits behind head on develop.

Files with missing lines Patch % Lines
...pache/rocketmq/remoting/protocol/LanguageCode.java 88.88% 0 Missing and 1 partial ⚠️
...ache/rocketmq/remoting/protocol/SerializeType.java 66.66% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@              Coverage Diff              @@
##             develop   #10513      +/-   ##
=============================================
- Coverage      48.06%   48.05%   -0.02%     
- Complexity     13313    13327      +14     
=============================================
  Files           1377     1377              
  Lines         100632   100711      +79     
  Branches       12995    13012      +17     
=============================================
+ Hits           48369    48396      +27     
- Misses         46344    46369      +25     
- Partials        5919     5946      +27     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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.

4 participants