Conversation
|
@copilot Can you measure whether this has any performance benefit compared to CI running on master? |
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Improves performance of boxed-primitive detection by avoiding expensive qualified-name extraction when the declared type’s simple name cannot match a boxed primitive.
Changes:
- Adds an early “fast reject” path based on
TypeElement+ simple-name checks before callingTypesUtils.getQualifiedName. - Introduces
isPossiblyBoxedSimpleName(Name)helper to centralize the simple-name comparisons.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Fast reject: all boxed primitives are in java.lang and have short names. | ||
| // A DeclaredType whose asElement is not a TypeElement, or whose simple name | ||
| // doesn't match one of 8 known short names, can't be boxed. |
There was a problem hiding this comment.
The comment implies the fast-reject validates “in java.lang”, but the code only checks the simple name and still relies on the later qualified-name lookup to confirm the package/class. To avoid misleading future maintainers, update the comment (and/or the helper Javadoc) to explicitly state this is a heuristic prefilter that does not validate the package.
| private static boolean isPossiblyBoxedSimpleName(Name n) { | ||
| return n.contentEquals("Byte") | ||
| || n.contentEquals("Boolean") | ||
| || n.contentEquals("Character") | ||
| || n.contentEquals("Double") | ||
| || n.contentEquals("Float") | ||
| || n.contentEquals("Integer") | ||
| || n.contentEquals("Long") | ||
| || n.contentEquals("Short"); | ||
| } |
There was a problem hiding this comment.
This hard-coded list duplicates information that already exists in the boxedToPrimitiveType mapping, creating a risk that the two fall out of sync if boxed types are ever adjusted. Consider deriving the simple-name allowlist from the existing map once (e.g., a static Set<String> of simple names built from the map’s keys), and have isPossiblyBoxedSimpleName query that set (still using Name.contentEquals against the set’s entries or by converting Name to String only after a cheap length/first-char gate).
I measured what is currently available from Actions. The master baseline at base SHA |
No description provided.