Obsolete warning: ITA_INEFFICIENT_TO_ARRAY #993
Comments
|
Thanks for opening your first issue here! 😃 |
|
read most of that but its more about knowing what data you are working with so GC doesn't happen right away. They seem to even imply that throughout. If you know your size, and I mean know it, then its better to size it correctly to start with. I don't think any scanners actually changed either in regards. Given 4 years now and no response here, going to close. If still a considered problem, please find new info on the subject as profiling can be hard. I don't see ramp ups to prove with certainty their data is 100% accurate and even some of their numbers are off a bit to their conclusions. When dealing with big record sets which is more the norm when copying, it still appears even on their own results that sizing it was better. Its the really small ones that is not true and that I'd agree since jvm is optimized for 16 by default regardless so less than that would go against its optimization and there is no example here in this issue to even state what type we were dealing with. At any rate, mostly closing as really old and no one responded. |
I don't understand this. I think what Aleksey showed is that zeroing out the newly allocated array takes time (an even more significant part of toArray(new T[size]) vs toArray(new T[0]) story is zeroing elimination), which is not related to GC. The GC aspect of the measurement is detailed here: https://shipilev.net/blog/2016/arrays-wisdom-ancients/#_not_an_allocation_pressure
The tests were run using JMH. The author of the article is one of the maintainers:
You can see the warm-up (is that what you mean by ramp-up?) in the benchmark section: @Warmup(iterations = 5, time = 1, timeUnit = TimeUnit.SECONDS)Or, if you meant measurements with an increasing array size: @Param({"0", "1", "10", "100", "1000"})
int size; |
|
Would want a longer warmup than 5 seconds. Also, would want to test on more recent jdk. That was on 9. It's also 7 years old and does not appear the tests were shared. Maybe they have it in jmh samples I would need to look.
As far as I'm aware. Arrays are 16 by default so the usage on 0 and 1 are clear without even checking what the data there states, so if spotbugs says to do something in those cases and it knows the size it would be incorrect but i dont think it can know. For 1000, it has to resize. So that eventually impacts GC which I didn't think that got into. Personally coming from platforms extremely sensitive to memory usage and poor GC performance, ie hp nonstop platforms, GC considerations matter. Maybe not as much today though then 10 years or longer ago.
At any rate main consideration on closing ticket here is that data is stale. No one tried to do anything with this. If you can find data to repeat tests and possible write up a change after seeing results it would be certainly considered. Otherwise closing issues is more about trying to gain some control of our repo as we gave well over 400 issues that are most likely never going to be addressed. I do admire your patience considering i expected no response on such an old issue.
Sent from my Verizon, Samsung Galaxy smartphone
Get Outlook for Android<https://aka.ms/AAb9ysg>
…________________________________
From: Kofa ***@***.***>
Sent: Sunday, August 20, 2023 3:56:42 AM
To: spotbugs/spotbugs ***@***.***>
Cc: Jeremy Landis ***@***.***>; State change ***@***.***>
Subject: Re: [spotbugs/spotbugs] Obsolete warning: ITA_INEFFICIENT_TO_ARRAY (#993)
its more about knowing what data you are working with so GC doesn't happen right away
I don't understand this. I think what Aleksey showed is that zeroing out the newly allocated array takes time (an even more significant part of toArray(new T[size]) vs toArray(new T[0]) story is zeroing elimination), which is not related to GC. The GC aspect of the measurement is detailed here: https://shipilev.net/blog/2016/arrays-wisdom-ancients/#_not_an_allocation_pressure
I don't see ramp ups to prove with certainty their data is 100% accurate
The tests were run using JMH. The author of the article is one of the maintainers:
Aleksey is working on Java performance for 15+ years. Today he is employed by AWS, where he does OpenJDK development and performance work. Aleksey develops and maintains a number of OpenJDK subprojects, including JMH, JOL, and JCStress.
(https://shipilev.net/)
You can see the warm-up (is that what you mean by ramp-up?) in the benchmark<https://shipilev.net/blog/2016/arrays-wisdom-ancients/#_benchmark> section:
@WarmUp(iterations = 5, time = 1, timeUnit = TimeUnit.SECONDS)
Or, if you meant measurements with an increasing array size:
@param({"0", "1", "10", "100", "1000"})
int size;
—
Reply to this email directly, view it on GitHub<#993 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AAHODI7QIDRVJRLJGH4OICLXWG7LVANCNFSM4IGP7BSQ>.
You are receiving this because you modified the open/close state.Message ID: ***@***.***>
|
|
reopening, see mybatis/mybatis-3#2925, has some more information on the subject. trade off on GC I noted is not worth the results. However, need a PR for this... |
https://spotbugs.readthedocs.io/en/stable/bugDescriptions.html#ita-method-uses-toarray-with-zero-length-array-argument-ita-inefficient-to-array
According to detailed analysis by Aleksey Shipilëv (https://shipilev.net/blog/2016/arrays-wisdom-ancients/), the zero-length initialisation has been faster than pre-sized since at least Java 8. IntelliJ Idea has been updated to recommend the use of the new style now (when using recent versions of Java), see https://github.com/JetBrains/intellij-community/blob/master/plugins/InspectionGadgets/src/inspectionDescriptions/ToArrayCallWithZeroLengthArrayArgument.html.
The text was updated successfully, but these errors were encountered: