[ISSUE #10464] Avoid unnecessary allocation in Message.getProperty and NFE in getPriority

[wang-jiahua]

Which Issue(s) This PR Fixes

Fixes #10464

Brief Description

Two per-message allocation/CPU fixes in Message.java:

1. getProperty(String): when properties == null, returns null immediately instead of creating a new HashMap<>() as a side-effect of a read-only call.

2. getPriority(): adds a null/empty fast-path before delegating to NumberUtils.toInt(). Previously, NumberUtils.toInt(null, -1) internally called Integer.parseInt(null) which throws and catches a NumberFormatException on every invocation (~16,000 NFE/min under benchmark load). The fast-path skips this throw+catch when PRIORITY is unset (the vast majority of messages).

Compatibility

• getProperty() no longer creates an empty HashMap as a side-effect. Callers that relied on this side-effect to later put directly into properties (without going through putProperty()) would break — but no such callers exist in the codebase (verified by grep).
• getPriority() return values are unchanged for all inputs.

How Did You Test This Change

mvn -pl common -Dcheckstyle.skip=false -Dspotbugs.skip=true validate
# 0 Checkstyle violations

Existing unit tests pass. The behavioral change in getProperty() is safe because hasProperty() already handles properties == null by returning false, and all write paths go through putProperty() which creates the HashMap on demand.

[Copilot]

Pull request overview

Note

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

Reduces side effects when reading message properties and makes priority parsing more explicit when the stored value is missing/empty.

Changes:

• Stop initializing properties on read in getProperty(...) (return null when properties is absent).
• Add explicit null/empty handling when parsing priority in getPriority().

---

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

[oss-sentinel-ai]

Review: Approved ✅

PR: #10468 — Avoid unnecessary allocation in Message.getProperty and NFE in getPriority
Type: Performance (1 file, +6/-3)

Assessment

Fixes two per-message allocation issues:

• getProperty() returns null immediately when properties is null (no HashMap creation)
• getPriority() adds null/empty fast-path (eliminates ~16,000 NFE/min)

Verdict

✅ Clean performance fix based on JFR profiling. Fixes #10464.

---

🤖 Automated review by oss-sentinel-ai

[RockteMQ-AI]

Review by github-manager-bot

Summary

Performance optimization in Message.java: eliminates unnecessary HashMap allocation in getProperty() and avoids NumberFormatException throw+catch in getPriority() when priority is unset.

Findings

• [Info] getProperty() — Removing the side-effect (creating HashMap on read) is correct. A getter should not mutate state. The Javadoc addition clearly documents the new contract.
• [Info] getPriority() — The null/empty fast-path before NumberUtils.toInt() is a clean fix. Under high throughput this avoids significant NFE overhead (~16k/min as noted).
• [Info] Compatibility — The author verified no callers depend on the HashMap creation side-effect. This is safe.

Suggestions

• Consider adding a unit test for getProperty() when properties == null to prevent regression of the side-effect behavior.
• Consider a microbenchmark (JMH) for getPriority() to quantify the NFE avoidance improvement.

Overall: Clean, well-documented performance fix. LGTM.

---

Automated review by github-manager-bot