Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CollectionToCollectionConverter.convert() improvement for untyped collections [SPR-11479] #16104

Closed
spring-projects-issues opened this issue Feb 24, 2014 · 2 comments
Assignees
Labels
in: core status: backported type: enhancement
Milestone

Comments

@spring-projects-issues
Copy link
Collaborator

@spring-projects-issues spring-projects-issues commented Feb 24, 2014

Gary Russell opened SPR-11479 and commented

Consider

  • source : ArrayList
  • sourceType : java.util.ArrayList<?>
  • targetType : java.util.Collection<?>

Line #63...

https://github.com/spring-projects/spring-framework/blob/master/spring-core/src/main/java/org/springframework/core/convert/support/CollectionToCollectionConverter.java#L63

...determines that copyRequired == false .

However, line 70 then proceeds to copy the elements from the source to the target, which is eventually discarded, when line #84 simply returns the source.

It looks to me like it should have

if (copyRequired) {} 
``` around the for loop. The else clause needs to run, in case `copyRequired` gets changed to true.

Affects: 3.2.8, 4.0.2

Issue Links:

  • INT-3306 Aggregator memory usage raises serious performance issue

Referenced from: commits bea94d5, d317b63

Backported to: 3.2.9

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Mar 5, 2014

Stéphane Nicoll commented

See PR: #483

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Mar 10, 2014

Juergen Hoeller commented

I've taken the tests from the pull request but eventually opted for a different implementation in CollectionToCollectionConverter, avoiding a collection copy for an untyped collection in the first place, and also avoiding repeated getElementTypeDescriptor calls (since that one re-introspects on every call).

The implementation also uses addAll instead of iteration over an untyped collection now, supporting a potentially optimized addAll implementation in the target collection type. No idea why that wasn't done years ago...

I've scheduled this for backporting to 3.2.9 as well, since the old behavior is suboptimal enough to become a performance concern in demanding scenarios with lots of on-the-fly collection conversions.

Juergen

@spring-projects-issues spring-projects-issues added status: backported type: enhancement in: core labels Jan 11, 2019
@spring-projects-issues spring-projects-issues added this to the 4.0.3 milestone Jan 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core status: backported type: enhancement
Projects
None yet
Development

No branches or pull requests

2 participants