Skip to content

Conversation

@stsypanov
Copy link
Contributor

Simple clean-up of reflection-related code: use ReflectionUtils.isEqualsMethod() / ReflectionUtils.isHashCodeMethod() / ReflectionUtils.isToStringMethod() in order to have unified approach in all pieces of code.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Dec 17, 2019
@jhoeller jhoeller self-assigned this Dec 18, 2019
@jhoeller
Copy link
Contributor

This is to some degree intentional since those ReflectionUtils methods implement strict signature checks (including parameter types) whereas the proxy-internal checks make simplified assumptions and just compare the method name if no same-named overloaded methods are to be expected. From that perspective, it seems sensible to retain the current simple checks in those places; I have yet to review whether the stricter checks might be applicable for certain code paths there.

@stsypanov
Copy link
Contributor Author

Oh, I see. I can then revert illegal changes and keep at least those in ReflectionUtils as comparison of int is likely to be faster than call to Object.equals()

@stsypanov
Copy link
Contributor Author

Damn, it seems I've force-pushed another branch into it... I'll rename PR then according to its purpose.

Here I've noticed that in numerous cases ClassUtils.toClassArray() is called with empty collection resulting in garbage Class[] allocations. This can be dodged in the similar way as we do in StringUtils.toStringArray()

@stsypanov stsypanov changed the title Use RU.isEqualsMethod() / RU.isHashCodeMethod() / RU.isToStringMethod() where appropriate Reduce Class[] garbage when creating proxies Mar 16, 2020
@jhoeller jhoeller added in: core Issues in core modules (aop, beans, core, context, expression) type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Mar 16, 2020
@jhoeller jhoeller added this to the 5.2.5 milestone Mar 16, 2020
@jhoeller
Copy link
Contributor

I'm applying a part of this for 5.2.5 still, namely a private EMPTY_CLASS_ARRAY constant that we're using in toClassArray, aligning it with the recent revision of StringUtils.toStringArray in the 5.2 line.

As for the code in ExtendedEntityManagerCreator, I'd rather prefer to keep it aligned with the multi-interface case below. Also, since ClassUtils.hasConstructor would require a new public method and isn't really used in a common hot code path, I'd rather keep the regular version only.

Thanks for the extensive pull request, in any case!

@jhoeller jhoeller closed this in 2209e7c Mar 23, 2020
@stsypanov stsypanov deleted the refl branch March 24, 2020 09:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

in: core Issues in core modules (aop, beans, core, context, expression) type: enhancement A general enhancement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants