Skip to content
This repository has been archived by the owner on Jan 13, 2023. It is now read-only.

Need precisions about JAVA S53 rule #176

Open
durandx opened this issue Nov 9, 2022 · 7 comments
Open

Need precisions about JAVA S53 rule #176

durandx opened this issue Nov 9, 2022 · 7 comments

Comments

@durandx
Copy link

durandx commented Nov 9, 2022

Hi,

I don't understand the recommandation concerning the rule cnumr-java:S53 (https://github.com/cnumr/ecoCode/blob/main/src/java-plugin/src/main/java/fr/cnumr/java/checks/UseCorrectForLoop.java)

Is it possible to access to the research paper regarding this rule (benchmark, java byte code analysis ...) ?
What are the experimental conditions ? Does the tests have been run on Android only, or does the tests have been on a server on Linux ?

@durandx durandx changed the title Need precisions about JAVA S53 rule ? Need precisions about JAVA S53 rule Nov 9, 2022
@olegoaer
Copy link
Member

I don't know where this rule comes from but it's for sure not part of the Android-specific plugin

@oussamaLaribi
Copy link
Collaborator

I think this rule has to do with Java performance with Collections. As i know, Foreach and Stream API is convenient to work with Collections.

@dedece35
Copy link
Collaborator

Hi @oussamaLaribi ,

I think @durandx would like to have an evidence to confirm what you said. I agree with @durandx. If we don't have a paper, it would b great to do benchmark test to check it (for example with JMH framework for example)
But this discussion goes is a general discussion : how to prove that each implemented rule is legitimate.
what's your opinion @olegoaer, @jules-delecour-dav, @glalloue, @mdubois81 ?

@oussamaLaribi
Copy link
Collaborator

Hello @dedece35, I have made a benchmark to test the example in the description of the rule. Here’s the code and the result. I'm kinda surprised.
Benchmark

ElapsedTime
https://github.com/oussamaLaribi/jmh-benchmark-JAVA-S53-rule

@dedece35
Copy link
Collaborator

Hi @oussamaLaribi ,

ok. good for micro-benchmark with JMH !
but In your code, there are some useless code. The unit test is useless for me, because benchmark is enough to prove it.

Another idea, is to benchmark other kind of loops. For example, to benchmark a stream, or standard for loop with simple index.

@oussamaLaribi
Copy link
Collaborator

Hi @dedece35,
thank you for ideas. I agree with you to benchmark other kind of loops.

@skerdudou
Copy link

Thank you @oussamaLaribi for the benchmark. So if I understand correctly, looping from an Array is quicker than from a List, the exact opposite that said by the S53 rule !

To be sure, I made some more tests with JMH.

I added a test where the Arrays.asList is not prebaked (as I would "correct" my code regarding this rule).
I also added a .stream().forEach as suggested (I'm curious too 😄).

With a big list (1_000_000 items) :

Benchmark                      Mode  Cnt     Score     Error  Units
BenchmarkForEach.forEachArray  avgt    5  2440,479 ±  24,270  us/op
BenchmarkForEach.forEachList   avgt    5  4681,401 ±  85,861  us/op
BenchmarkForEach.forEachList2  avgt    5  4722,169 ±  99,151  us/op
BenchmarkForEach.stream        avgt    5  4403,399 ± 329,694  us/op

With a small list (10 items) :

Benchmark                      Mode  Cnt  Score   Error  Units
BenchmarkForEach.forEachArray  avgt    5  0,027 ± 0,002  us/op
BenchmarkForEach.forEachList   avgt    5  0,038 ± 0,002  us/op
BenchmarkForEach.forEachList2  avgt    5  0,042 ± 0,003  us/op
BenchmarkForEach.stream        avgt    5  0,038 ± 0,002  us/op

With these tests I constantly have better performance on arrays...

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants