Replace legacy for loop with foreach in MappingMongoConverter [DATAMONGO-1449]

[spring-projects-issues]

Kevin Dosey opened DATAMONGO-1449 and commented

I'd like to propose the following change to get a minor performance improvement where I've observed a bottleneck when converting DBObjects to domain objects.

-		for (int i = 0; i < sourceValue.size(); i++) {
-
-			Object dbObjItem = sourceValue.get(i);
+		for (Object dbObjItem : sourceValue) {

---

Affects: 1.9.1 (Hopper SR1)

Reference URL:

spring-data-mongodb/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/MappingMongoConverter.java

Line 904 in 7b8dade

for (int i = 0; i < sourceValue.size(); i++) {

Referenced from: pull request #368

Backported to: 1.9.2 (Hopper SR2), 1.8.5 (Gosling SR5)

[spring-projects-issues]

Oliver Drotbohm commented

Is there any measurable proof that this makes any difference in real world scenarios? As far as I remember the foreach loop compiles to the code currently in place anyway. I am all in for stylistic improvements but doubt there's a real performance benefit. If so, I'd love to learn abou it :).

Also, it'd be great if you could point out where said loop is actually located in the code base

[spring-projects-issues]

Oliver Drotbohm commented

Regarding the location, nevermind. I see you linked to it.

[spring-projects-issues]

Kevin Dosey commented

I've come across this at work, and I'm unable to give you a direct example from there, but I can try to reproduce the situation where this occurs if necessary. I was just running jVisualVM trying to investigate why it was taking a while to generate the data for a web page that was performing poorly, and this was one of the minor hot spots I found. I copied the class and changed our code to reference the copied class, and this change and I shaved off a second out of roughly 30 seconds. It's not significant but every little bit helps

[spring-projects-issues]

Oliver Drotbohm commented

There's no need for a dedicated investigation as I think the change makes sense from a stylistic point of view alone. I was just kind of puzzled that reading nested lists becomes a performance bottle neck as very large ones are becoming a problem within MongoDB in the first place and it's generally recommended to not let them grow a lot or in a sort of unbounded way

[spring-projects-issues]

Kevin Dosey commented

Yeah, I kind of figured it would be better style wise as well so I didn't think the change would be controversial. I realize that this is not a significant bottleneck and it's more so that when you a have a large number of objects where each has a small embedded array that you are converting it slows down creating those objects just a bit. It just saves a little on reiterating through the list on each .get(i) call.

Thanks for considering my change

[spring-projects-issues]

Oliver Drotbohm commented

That's merged, thanks!