[infinispan-dev] To Optional or not to Optional?

classic Classic list List threaded Threaded
15 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[infinispan-dev] To Optional or not to Optional?

Sebastian Laskawiec
Hey!

In our past we had a couple of discussions about whether we should or should not use Optionals [1][2]. The main argument against it was performance. 

On one hand we risk additional object allocation (the Optional itself) and wrong inlining decisions taken by C2 compiler [3]. On the other hand we all probably "feel" that both of those things shouldn't be a problem and should be optimized by C2. Another argument was the Optional's doesn't give us anything but as I checked, we introduced nearly 80 NullPointerException bugs in two years [4]. So we might consider Optional as a way of fighting those things. The final argument that I've seen was about lack of higher order functions which is simply not true since we have #map, #filter and #flatmap functions. You can do pretty amazing things with this.

I decided to check the performance when refactoring REST interface. I created a PR with Optionals [5], ran performance tests, removed all Optionals and reran tests. You will be surprised by the results [6]:

Test case
With Optionals [%]Without Optionals
Run 1Run 2AvgRun 1Run 2Avg
Non-TX reads 10 threads
Throughput32.5432.8732.7131.7434.0432.89
Response time-24.12-24.63-24.38-24.37-25.69-25.03
Non-TX reads 100 threads
Throughput6.48-12.79-3.16-7.06-6.14-6.60
Response time-6.1514.934.397.886.497.19
Non-TX writes 10 threads
Throughput9.217.608.414.667.155.91
Response time-8.92-7.11-8.02-5.29-6.93-6.11
Non-TX writes 100 threads
Throughput2.531.652.09-1.164.671.76
Response time-2.13-1.79-1.960.91-4.67-1.88

I also created JMH + Flight Recorder tests and again, the results showed no evidence of slow down caused by Optionals [7].

Now please take those results with a grain of salt since they tend to drift by a factor of +/-5% (sometimes even more). But it's very clear the performance results are very similar if not the same.

Having those numbers at hand, do we want to have Optionals in Infinispan codebase or not? And if not, let's state it very clearly (and write it into contributing guide), it's because we don't like them. Not because of performance.

Thanks,
Sebastian

--

SEBASTIAN ŁASKAWIEC

INFINISPAN DEVELOPER


_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [infinispan-dev] To Optional or not to Optional?

Katia Aresti
Hi Sebastien,

First thing to say, you impress me with all this work ! Thank you very much !

I've been working with Scala for almost three years, and I really appreciate the functional code style. This involves the use of Optionals among other things you mention like map, flapMap etc. Looking at the performance test, it seems Optionals are not an issue, so it's more a matter of coding style and design in most of the cases.
After my experience with scala, I believe that if Optional do indeed avoid NullpointerException, they introduce NoSuchElemenException. Because the coding style with functional programming is more than a matter of Optionals Yes or No. It's very different from imperative programming and this will be hard to do really "as it should" in infinispan code base. So in the end, there will be moments where we will be calling "get" to an empty Optional, leading to the kind of bugs you listed before involving NullPointerException. At least, this is the case in my experience, specially coming from years of Java coding and with such a huge code base. 
But I think it's good to use Optionals, specially on return types (not in method parameters). 

So 1+ to use Optionals, and  +1 to decide clearly how and when and following which coding style rules we should introduce them in our public APIs, internal APIs and codebase in general.

My 2 cents,

Katia




On Thu, May 18, 2017 at 1:30 PM, Sebastian Laskawiec <[hidden email]> wrote:
Hey!

In our past we had a couple of discussions about whether we should or should not use Optionals [1][2]. The main argument against it was performance. 

On one hand we risk additional object allocation (the Optional itself) and wrong inlining decisions taken by C2 compiler [3]. On the other hand we all probably "feel" that both of those things shouldn't be a problem and should be optimized by C2. Another argument was the Optional's doesn't give us anything but as I checked, we introduced nearly 80 NullPointerException bugs in two years [4]. So we might consider Optional as a way of fighting those things. The final argument that I've seen was about lack of higher order functions which is simply not true since we have #map, #filter and #flatmap functions. You can do pretty amazing things with this.

I decided to check the performance when refactoring REST interface. I created a PR with Optionals [5], ran performance tests, removed all Optionals and reran tests. You will be surprised by the results [6]:

Test case
With Optionals [%]Without Optionals
Run 1Run 2AvgRun 1Run 2Avg
Non-TX reads 10 threads
Throughput32.5432.8732.7131.7434.0432.89
Response time-24.12-24.63-24.38-24.37-25.69-25.03
Non-TX reads 100 threads
Throughput6.48-12.79-3.16-7.06-6.14-6.60
Response time-6.1514.934.397.886.497.19
Non-TX writes 10 threads
Throughput9.217.608.414.667.155.91
Response time-8.92-7.11-8.02-5.29-6.93-6.11
Non-TX writes 100 threads
Throughput2.531.652.09-1.164.671.76
Response time-2.13-1.79-1.960.91-4.67-1.88

I also created JMH + Flight Recorder tests and again, the results showed no evidence of slow down caused by Optionals [7].

Now please take those results with a grain of salt since they tend to drift by a factor of +/-5% (sometimes even more). But it's very clear the performance results are very similar if not the same.

Having those numbers at hand, do we want to have Optionals in Infinispan codebase or not? And if not, let's state it very clearly (and write it into contributing guide), it's because we don't like them. Not because of performance.

Thanks,
Sebastian

--

SEBASTIAN ŁASKAWIEC

INFINISPAN DEVELOPER


_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev


_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [infinispan-dev] To Optional or not to Optional?

Sanne Grinovero-3
In reply to this post by Sebastian Laskawiec
Hi Sebastian,

sorry but I think you've been wasting time, I hope it was fun :) This is not the right methodology to "settle" the matter (unless you want Radim's eyes to get bloody..).

Any change in such a complex system will only affect the performance metrics if you're actually addressing the dominant bottleneck. In some cases it might be CPU, like if your system is at 90%+ CPU then it's likely that reviewing the code to use less CPU would be beneficial; but even that can be counter-productive, for example if you're having contention caused by optimistic locking and you fail to address that while making something else "faster" the performance loss on the optimistic lock might become asymptotic.

A good reason to avoid excessive usage of Optional (and *excessive* doesn't mean a couple dozen in a millions lines of code..) is to not run out of eden space, especially for all the code running in interpreted mode.

In your case you've been benchmarking a hugely complex beast, not least over REST! When running the REST Server I doubt that allocation in eden is your main problem. You just happened to have a couple Optionals on your path; sure performance changed but there's no enough data in this way to figure out what exactly happened:
 - did it change at all or was it just because of a lucky optimisation? (The JIT will always optimise stuff differently even when re-running the same code)
 - did the overall picture improve because this code became much *less* slower?

The real complexity in benchmarking is to accurately understand why it changed; this should also tell you why it didn't change more, or less..

To be fair I actually agree that it's very likely that C2 can make any performance penalty disappear.. that's totally possible, although it's unlikely to be faster than just reading the field (assuming we don't need to do branching because of null-checks but C2 can optimise that as well).
Still this requires the code to be optimised by JIT first, so it won't prevent us from creating a gazillion of instances if we abuse its usage irresponsibly. Fighting internal NPEs is a matter of writing better code; I'm not against some "Optional" being strategically placed but I believe it's much nicer for most internal code to just avoid null, use "final", and initialize things aggressively.

Sure use Optional where it makes sense, probably most on APIs and SPIs, but please don't go overboard with it in internals. That's all I said in the original debate.

In case you want to benchmark the impact of Optional make a JMH based microbenchmark - that's interesting to see what C2 is capable of - but even so that's not going to tell you much on the impact it would have to patch thousands of code all around Infinispan. And it will need some peer review before it can tell you anything at all ;)

It's actually a very challenging topic, as we produce libraries meant for "anyone to use" and don't get to set the hardware specification requirements it's hard to predict if we should optimise the system for this/that resource consumption. Some people will have plenty of CPU and have problems with us needing too much memory, some others will have the opposite.. the real challenge is in making internals "elastic" to such factors and adaptable without making it too hard to tune.

Thanks,
Sanne



On 18 May 2017 at 12:30, Sebastian Laskawiec <[hidden email]> wrote:
Hey!

In our past we had a couple of discussions about whether we should or should not use Optionals [1][2]. The main argument against it was performance. 

On one hand we risk additional object allocation (the Optional itself) and wrong inlining decisions taken by C2 compiler [3]. On the other hand we all probably "feel" that both of those things shouldn't be a problem and should be optimized by C2. Another argument was the Optional's doesn't give us anything but as I checked, we introduced nearly 80 NullPointerException bugs in two years [4]. So we might consider Optional as a way of fighting those things. The final argument that I've seen was about lack of higher order functions which is simply not true since we have #map, #filter and #flatmap functions. You can do pretty amazing things with this.

I decided to check the performance when refactoring REST interface. I created a PR with Optionals [5], ran performance tests, removed all Optionals and reran tests. You will be surprised by the results [6]:

Test case
With Optionals [%]Without Optionals
Run 1Run 2AvgRun 1Run 2Avg
Non-TX reads 10 threads
Throughput32.5432.8732.7131.7434.0432.89
Response time-24.12-24.63-24.38-24.37-25.69-25.03
Non-TX reads 100 threads
Throughput6.48-12.79-3.16-7.06-6.14-6.60
Response time-6.1514.934.397.886.497.19
Non-TX writes 10 threads
Throughput9.217.608.414.667.155.91
Response time-8.92-7.11-8.02-5.29-6.93-6.11
Non-TX writes 100 threads
Throughput2.531.652.09-1.164.671.76
Response time-2.13-1.79-1.960.91-4.67-1.88

I also created JMH + Flight Recorder tests and again, the results showed no evidence of slow down caused by Optionals [7].

Now please take those results with a grain of salt since they tend to drift by a factor of +/-5% (sometimes even more). But it's very clear the performance results are very similar if not the same.

Having those numbers at hand, do we want to have Optionals in Infinispan codebase or not? And if not, let's state it very clearly (and write it into contributing guide), it's because we don't like them. Not because of performance.

Thanks,
Sebastian

--

SEBASTIAN ŁASKAWIEC

INFINISPAN DEVELOPER


_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev


_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [infinispan-dev] To Optional or not to Optional?

Galder Zamarreño
I think Sanne's right here, any differences in such large scale test are hard to decipher.

Also, as mentioned in a previous email, my view on its usage is same as Sanne's:

* Definitely in APIs/SPIs.
* Be gentle with it internals.

Cheers,
--
Galder Zamarreño
Infinispan, Red Hat

> On 18 May 2017, at 14:35, Sanne Grinovero <[hidden email]> wrote:
>
> Hi Sebastian,
>
> sorry but I think you've been wasting time, I hope it was fun :) This is not the right methodology to "settle" the matter (unless you want Radim's eyes to get bloody..).
>
> Any change in such a complex system will only affect the performance metrics if you're actually addressing the dominant bottleneck. In some cases it might be CPU, like if your system is at 90%+ CPU then it's likely that reviewing the code to use less CPU would be beneficial; but even that can be counter-productive, for example if you're having contention caused by optimistic locking and you fail to address that while making something else "faster" the performance loss on the optimistic lock might become asymptotic.
>
> A good reason to avoid excessive usage of Optional (and *excessive* doesn't mean a couple dozen in a millions lines of code..) is to not run out of eden space, especially for all the code running in interpreted mode.
>
> In your case you've been benchmarking a hugely complex beast, not least over REST! When running the REST Server I doubt that allocation in eden is your main problem. You just happened to have a couple Optionals on your path; sure performance changed but there's no enough data in this way to figure out what exactly happened:
>  - did it change at all or was it just because of a lucky optimisation? (The JIT will always optimise stuff differently even when re-running the same code)
>  - did the overall picture improve because this code became much *less* slower?
>
> The real complexity in benchmarking is to accurately understand why it changed; this should also tell you why it didn't change more, or less..
>
> To be fair I actually agree that it's very likely that C2 can make any performance penalty disappear.. that's totally possible, although it's unlikely to be faster than just reading the field (assuming we don't need to do branching because of null-checks but C2 can optimise that as well).
> Still this requires the code to be optimised by JIT first, so it won't prevent us from creating a gazillion of instances if we abuse its usage irresponsibly. Fighting internal NPEs is a matter of writing better code; I'm not against some "Optional" being strategically placed but I believe it's much nicer for most internal code to just avoid null, use "final", and initialize things aggressively.
>
> Sure use Optional where it makes sense, probably most on APIs and SPIs, but please don't go overboard with it in internals. That's all I said in the original debate.
>
> In case you want to benchmark the impact of Optional make a JMH based microbenchmark - that's interesting to see what C2 is capable of - but even so that's not going to tell you much on the impact it would have to patch thousands of code all around Infinispan. And it will need some peer review before it can tell you anything at all ;)
>
> It's actually a very challenging topic, as we produce libraries meant for "anyone to use" and don't get to set the hardware specification requirements it's hard to predict if we should optimise the system for this/that resource consumption. Some people will have plenty of CPU and have problems with us needing too much memory, some others will have the opposite.. the real challenge is in making internals "elastic" to such factors and adaptable without making it too hard to tune.
>
> Thanks,
> Sanne
>
>
>
> On 18 May 2017 at 12:30, Sebastian Laskawiec <[hidden email]> wrote:
> Hey!
>
> In our past we had a couple of discussions about whether we should or should not use Optionals [1][2]. The main argument against it was performance.
>
> On one hand we risk additional object allocation (the Optional itself) and wrong inlining decisions taken by C2 compiler [3]. On the other hand we all probably "feel" that both of those things shouldn't be a problem and should be optimized by C2. Another argument was the Optional's doesn't give us anything but as I checked, we introduced nearly 80 NullPointerException bugs in two years [4]. So we might consider Optional as a way of fighting those things. The final argument that I've seen was about lack of higher order functions which is simply not true since we have #map, #filter and #flatmap functions. You can do pretty amazing things with this.
>
> I decided to check the performance when refactoring REST interface. I created a PR with Optionals [5], ran performance tests, removed all Optionals and reran tests. You will be surprised by the results [6]:
>
> Test case
> With Optionals [%] Without Optionals
> Run 1 Run 2 Avg Run 1 Run 2 Avg
> Non-TX reads 10 threads
> Throughput 32.54 32.87 32.71 31.74 34.04 32.89
> Response time -24.12 -24.63 -24.38 -24.37 -25.69 -25.03
> Non-TX reads 100 threads
> Throughput 6.48 -12.79 -3.16 -7.06 -6.14 -6.60
> Response time -6.15 14.93 4.39 7.88 6.49 7.19
> Non-TX writes 10 threads
> Throughput 9.21 7.60 8.41 4.66 7.15 5.91
> Response time -8.92 -7.11 -8.02 -5.29 -6.93 -6.11
> Non-TX writes 100 threads
> Throughput 2.53 1.65 2.09 -1.16 4.67 1.76
> Response time -2.13 -1.79 -1.96 0.91 -4.67 -1.88
>
> I also created JMH + Flight Recorder tests and again, the results showed no evidence of slow down caused by Optionals [7].
>
> Now please take those results with a grain of salt since they tend to drift by a factor of +/-5% (sometimes even more). But it's very clear the performance results are very similar if not the same.
>
> Having those numbers at hand, do we want to have Optionals in Infinispan codebase or not? And if not, let's state it very clearly (and write it into contributing guide), it's because we don't like them. Not because of performance.
>
> Thanks,
> Sebastian
>
> [1] http://lists.jboss.org/pipermail/infinispan-dev/2017-March/017370.html
> [2] http://lists.jboss.org/pipermail/infinispan-dev/2016-August/016796.html
> [3] http://vanillajava.blogspot.ro/2015/01/java-lambdas-and-low-latency.html
> [4] https://issues.jboss.org/issues/?jql=project%20%3D%20ISPN%20AND%20issuetype%20%3D%20Bug%20AND%20text%20%7E%20%22NullPointerException%22%20AND%20created%20%3E%3D%202015-04-27%20AND%20created%20%3C%3D%202017-04-27
> [5] https://github.com/infinispan/infinispan/pull/5094
> [6] https://docs.google.com/a/redhat.com/spreadsheets/d/1oep6Was0FfvHdqBCwpCFIqcPfJZ5-5_YYUqlRtUxEkM/edit?usp=sharing
> [7] https://github.com/infinispan/infinispan/pull/5094#issuecomment-296970673
> --
> SEBASTIAN ŁASKAWIEC
> INFINISPAN DEVELOPER
> Red Hat EMEA
>
>
> _______________________________________________
> infinispan-dev mailing list
> [hidden email]
> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>
> _______________________________________________
> infinispan-dev mailing list
> [hidden email]
> https://lists.jboss.org/mailman/listinfo/infinispan-dev


_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [infinispan-dev] To Optional or not to Optional?

Sebastian Laskawiec
Hey!

So I think we have no extreme naysayers to Optional. So let me try to sum up what we have achieved so:
  • In macroscale benchmark based on REST interface using Optionals didn't lower the performance.
  • +1 for using it in public APIs, especially for those using functional style. 
  • Creating lots of Optional instances might add some pressure on GC, so we need to be careful when using them in hot code paths. In such cases it is required to run a micro scale benchamark to make sure the performance didn't drop. The microbenchmark should also be followed by macro scale benchamrk - PerfJobAck. Also, keep an eye on Eden space in such cases.
If you agree with me, and there are no hard evidence that using Optional degrade performance significantly, I would like to issue a pull request and put those findings into contributing guide [1].

Thanks,
Sebastian


On Mon, May 22, 2017 at 6:36 PM Galder Zamarreño <[hidden email]> wrote:
I think Sanne's right here, any differences in such large scale test are hard to decipher.

Also, as mentioned in a previous email, my view on its usage is same as Sanne's:

* Definitely in APIs/SPIs.
* Be gentle with it internals.

Cheers,
--
Galder Zamarreño
Infinispan, Red Hat

> On 18 May 2017, at 14:35, Sanne Grinovero <[hidden email]> wrote:
>
> Hi Sebastian,
>
> sorry but I think you've been wasting time, I hope it was fun :) This is not the right methodology to "settle" the matter (unless you want Radim's eyes to get bloody..).
>
> Any change in such a complex system will only affect the performance metrics if you're actually addressing the dominant bottleneck. In some cases it might be CPU, like if your system is at 90%+ CPU then it's likely that reviewing the code to use less CPU would be beneficial; but even that can be counter-productive, for example if you're having contention caused by optimistic locking and you fail to address that while making something else "faster" the performance loss on the optimistic lock might become asymptotic.
>
> A good reason to avoid excessive usage of Optional (and *excessive* doesn't mean a couple dozen in a millions lines of code..) is to not run out of eden space, especially for all the code running in interpreted mode.
>
> In your case you've been benchmarking a hugely complex beast, not least over REST! When running the REST Server I doubt that allocation in eden is your main problem. You just happened to have a couple Optionals on your path; sure performance changed but there's no enough data in this way to figure out what exactly happened:
>  - did it change at all or was it just because of a lucky optimisation? (The JIT will always optimise stuff differently even when re-running the same code)
>  - did the overall picture improve because this code became much *less* slower?
>
> The real complexity in benchmarking is to accurately understand why it changed; this should also tell you why it didn't change more, or less..
>
> To be fair I actually agree that it's very likely that C2 can make any performance penalty disappear.. that's totally possible, although it's unlikely to be faster than just reading the field (assuming we don't need to do branching because of null-checks but C2 can optimise that as well).
> Still this requires the code to be optimised by JIT first, so it won't prevent us from creating a gazillion of instances if we abuse its usage irresponsibly. Fighting internal NPEs is a matter of writing better code; I'm not against some "Optional" being strategically placed but I believe it's much nicer for most internal code to just avoid null, use "final", and initialize things aggressively.
>
> Sure use Optional where it makes sense, probably most on APIs and SPIs, but please don't go overboard with it in internals. That's all I said in the original debate.
>
> In case you want to benchmark the impact of Optional make a JMH based microbenchmark - that's interesting to see what C2 is capable of - but even so that's not going to tell you much on the impact it would have to patch thousands of code all around Infinispan. And it will need some peer review before it can tell you anything at all ;)
>
> It's actually a very challenging topic, as we produce libraries meant for "anyone to use" and don't get to set the hardware specification requirements it's hard to predict if we should optimise the system for this/that resource consumption. Some people will have plenty of CPU and have problems with us needing too much memory, some others will have the opposite.. the real challenge is in making internals "elastic" to such factors and adaptable without making it too hard to tune.
>
> Thanks,
> Sanne
>
>
>
> On 18 May 2017 at 12:30, Sebastian Laskawiec <[hidden email]> wrote:
> Hey!
>
> In our past we had a couple of discussions about whether we should or should not use Optionals [1][2]. The main argument against it was performance.
>
> On one hand we risk additional object allocation (the Optional itself) and wrong inlining decisions taken by C2 compiler [3]. On the other hand we all probably "feel" that both of those things shouldn't be a problem and should be optimized by C2. Another argument was the Optional's doesn't give us anything but as I checked, we introduced nearly 80 NullPointerException bugs in two years [4]. So we might consider Optional as a way of fighting those things. The final argument that I've seen was about lack of higher order functions which is simply not true since we have #map, #filter and #flatmap functions. You can do pretty amazing things with this.
>
> I decided to check the performance when refactoring REST interface. I created a PR with Optionals [5], ran performance tests, removed all Optionals and reran tests. You will be surprised by the results [6]:
>
> Test case
> With Optionals [%]    Without Optionals
> Run 1 Run 2   Avg     Run 1   Run 2   Avg
> Non-TX reads 10 threads
> Throughput    32.54   32.87   32.71   31.74   34.04   32.89
> Response time -24.12  -24.63  -24.38  -24.37  -25.69  -25.03
> Non-TX reads 100 threads
> Throughput    6.48    -12.79  -3.16   -7.06   -6.14   -6.60
> Response time -6.15   14.93   4.39    7.88    6.49    7.19
> Non-TX writes 10 threads
> Throughput    9.21    7.60    8.41    4.66    7.15    5.91
> Response time -8.92   -7.11   -8.02   -5.29   -6.93   -6.11
> Non-TX writes 100 threads
> Throughput    2.53    1.65    2.09    -1.16   4.67    1.76
> Response time -2.13   -1.79   -1.96   0.91    -4.67   -1.88
>
> I also created JMH + Flight Recorder tests and again, the results showed no evidence of slow down caused by Optionals [7].
>
> Now please take those results with a grain of salt since they tend to drift by a factor of +/-5% (sometimes even more). But it's very clear the performance results are very similar if not the same.
>
> Having those numbers at hand, do we want to have Optionals in Infinispan codebase or not? And if not, let's state it very clearly (and write it into contributing guide), it's because we don't like them. Not because of performance.
>
> Thanks,
> Sebastian
>
> [1] http://lists.jboss.org/pipermail/infinispan-dev/2017-March/017370.html
> [2] http://lists.jboss.org/pipermail/infinispan-dev/2016-August/016796.html
> [3] http://vanillajava.blogspot.ro/2015/01/java-lambdas-and-low-latency.html
> [4] https://issues.jboss.org/issues/?jql=project%20%3D%20ISPN%20AND%20issuetype%20%3D%20Bug%20AND%20text%20%7E%20%22NullPointerException%22%20AND%20created%20%3E%3D%202015-04-27%20AND%20created%20%3C%3D%202017-04-27
> [5] https://github.com/infinispan/infinispan/pull/5094
> [6] https://docs.google.com/a/redhat.com/spreadsheets/d/1oep6Was0FfvHdqBCwpCFIqcPfJZ5-5_YYUqlRtUxEkM/edit?usp=sharing
> [7] https://github.com/infinispan/infinispan/pull/5094#issuecomment-296970673
> --
> SEBASTIAN ŁASKAWIEC
> INFINISPAN DEVELOPER
> Red Hat EMEA
>
>
> _______________________________________________
> infinispan-dev mailing list
> [hidden email]
> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>
> _______________________________________________
> infinispan-dev mailing list
> [hidden email]
> https://lists.jboss.org/mailman/listinfo/infinispan-dev


_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev
--

SEBASTIAN ŁASKAWIEC

INFINISPAN DEVELOPER


_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [infinispan-dev] To Optional or not to Optional?

Dan Berindei
I wouldn't say I'm an extreme naysayer, but I do have 2 issues with Optional:

1. Performance becomes harder to quantify: the allocations may or may not be eliminated, and a change in one part of the code may change how allocations are eliminated in a completely different part of the code.
2. My personal opinion is it's just ugly... instead of having one field that could be null or non-null, you now have a field that could be null, Optional.empty(), or Optional.of(something).

Cheers
Dan


On Tue, May 23, 2017 at 1:54 PM, Sebastian Laskawiec <[hidden email]> wrote:
Hey!

So I think we have no extreme naysayers to Optional. So let me try to sum up what we have achieved so:
  • In macroscale benchmark based on REST interface using Optionals didn't lower the performance.
  • +1 for using it in public APIs, especially for those using functional style. 
  • Creating lots of Optional instances might add some pressure on GC, so we need to be careful when using them in hot code paths. In such cases it is required to run a micro scale benchamark to make sure the performance didn't drop. The microbenchmark should also be followed by macro scale benchamrk - PerfJobAck. Also, keep an eye on Eden space in such cases.
If you agree with me, and there are no hard evidence that using Optional degrade performance significantly, I would like to issue a pull request and put those findings into contributing guide [1].

Thanks,
Sebastian


On Mon, May 22, 2017 at 6:36 PM Galder Zamarreño <[hidden email]> wrote:
I think Sanne's right here, any differences in such large scale test are hard to decipher.

Also, as mentioned in a previous email, my view on its usage is same as Sanne's:

* Definitely in APIs/SPIs.
* Be gentle with it internals.

Cheers,
--
Galder Zamarreño
Infinispan, Red Hat

> On 18 May 2017, at 14:35, Sanne Grinovero <[hidden email]> wrote:
>
> Hi Sebastian,
>
> sorry but I think you've been wasting time, I hope it was fun :) This is not the right methodology to "settle" the matter (unless you want Radim's eyes to get bloody..).
>
> Any change in such a complex system will only affect the performance metrics if you're actually addressing the dominant bottleneck. In some cases it might be CPU, like if your system is at 90%+ CPU then it's likely that reviewing the code to use less CPU would be beneficial; but even that can be counter-productive, for example if you're having contention caused by optimistic locking and you fail to address that while making something else "faster" the performance loss on the optimistic lock might become asymptotic.
>
> A good reason to avoid excessive usage of Optional (and *excessive* doesn't mean a couple dozen in a millions lines of code..) is to not run out of eden space, especially for all the code running in interpreted mode.
>
> In your case you've been benchmarking a hugely complex beast, not least over REST! When running the REST Server I doubt that allocation in eden is your main problem. You just happened to have a couple Optionals on your path; sure performance changed but there's no enough data in this way to figure out what exactly happened:
>  - did it change at all or was it just because of a lucky optimisation? (The JIT will always optimise stuff differently even when re-running the same code)
>  - did the overall picture improve because this code became much *less* slower?
>
> The real complexity in benchmarking is to accurately understand why it changed; this should also tell you why it didn't change more, or less..
>
> To be fair I actually agree that it's very likely that C2 can make any performance penalty disappear.. that's totally possible, although it's unlikely to be faster than just reading the field (assuming we don't need to do branching because of null-checks but C2 can optimise that as well).
> Still this requires the code to be optimised by JIT first, so it won't prevent us from creating a gazillion of instances if we abuse its usage irresponsibly. Fighting internal NPEs is a matter of writing better code; I'm not against some "Optional" being strategically placed but I believe it's much nicer for most internal code to just avoid null, use "final", and initialize things aggressively.
>
> Sure use Optional where it makes sense, probably most on APIs and SPIs, but please don't go overboard with it in internals. That's all I said in the original debate.
>
> In case you want to benchmark the impact of Optional make a JMH based microbenchmark - that's interesting to see what C2 is capable of - but even so that's not going to tell you much on the impact it would have to patch thousands of code all around Infinispan. And it will need some peer review before it can tell you anything at all ;)
>
> It's actually a very challenging topic, as we produce libraries meant for "anyone to use" and don't get to set the hardware specification requirements it's hard to predict if we should optimise the system for this/that resource consumption. Some people will have plenty of CPU and have problems with us needing too much memory, some others will have the opposite.. the real challenge is in making internals "elastic" to such factors and adaptable without making it too hard to tune.
>
> Thanks,
> Sanne
>
>
>
> On 18 May 2017 at 12:30, Sebastian Laskawiec <[hidden email]> wrote:
> Hey!
>
> In our past we had a couple of discussions about whether we should or should not use Optionals [1][2]. The main argument against it was performance.
>
> On one hand we risk additional object allocation (the Optional itself) and wrong inlining decisions taken by C2 compiler [3]. On the other hand we all probably "feel" that both of those things shouldn't be a problem and should be optimized by C2. Another argument was the Optional's doesn't give us anything but as I checked, we introduced nearly 80 NullPointerException bugs in two years [4]. So we might consider Optional as a way of fighting those things. The final argument that I've seen was about lack of higher order functions which is simply not true since we have #map, #filter and #flatmap functions. You can do pretty amazing things with this.
>
> I decided to check the performance when refactoring REST interface. I created a PR with Optionals [5], ran performance tests, removed all Optionals and reran tests. You will be surprised by the results [6]:
>
> Test case
> With Optionals [%]    Without Optionals
> Run 1 Run 2   Avg     Run 1   Run 2   Avg
> Non-TX reads 10 threads
> Throughput    32.54   32.87   32.71   31.74   34.04   32.89
> Response time -24.12  -24.63  -24.38  -24.37  -25.69  -25.03
> Non-TX reads 100 threads
> Throughput    6.48    -12.79  -3.16   -7.06   -6.14   -6.60
> Response time -6.15   14.93   4.39    7.88    6.49    7.19
> Non-TX writes 10 threads
> Throughput    9.21    7.60    8.41    4.66    7.15    5.91
> Response time -8.92   -7.11   -8.02   -5.29   -6.93   -6.11
> Non-TX writes 100 threads
> Throughput    2.53    1.65    2.09    -1.16   4.67    1.76
> Response time -2.13   -1.79   -1.96   0.91    -4.67   -1.88
>
> I also created JMH + Flight Recorder tests and again, the results showed no evidence of slow down caused by Optionals [7].
>
> Now please take those results with a grain of salt since they tend to drift by a factor of +/-5% (sometimes even more). But it's very clear the performance results are very similar if not the same.
>
> Having those numbers at hand, do we want to have Optionals in Infinispan codebase or not? And if not, let's state it very clearly (and write it into contributing guide), it's because we don't like them. Not because of performance.
>
> Thanks,
> Sebastian
>
> [1] http://lists.jboss.org/pipermail/infinispan-dev/2017-March/017370.html
> [2] http://lists.jboss.org/pipermail/infinispan-dev/2016-August/016796.html
> [3] http://vanillajava.blogspot.ro/2015/01/java-lambdas-and-low-latency.html
> [4] https://issues.jboss.org/issues/?jql=project%20%3D%20ISPN%20AND%20issuetype%20%3D%20Bug%20AND%20text%20%7E%20%22NullPointerException%22%20AND%20created%20%3E%3D%202015-04-27%20AND%20created%20%3C%3D%202017-04-27
> [5] https://github.com/infinispan/infinispan/pull/5094
> [6] https://docs.google.com/a/redhat.com/spreadsheets/d/1oep6Was0FfvHdqBCwpCFIqcPfJZ5-5_YYUqlRtUxEkM/edit?usp=sharing
> [7] https://github.com/infinispan/infinispan/pull/5094#issuecomment-296970673
> --
> SEBASTIAN ŁASKAWIEC
> INFINISPAN DEVELOPER
> Red Hat EMEA
>
>
> _______________________________________________
> infinispan-dev mailing list
> [hidden email]
> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>
> _______________________________________________
> infinispan-dev mailing list
> [hidden email]
> https://lists.jboss.org/mailman/listinfo/infinispan-dev


_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev
--

SEBASTIAN ŁASKAWIEC

INFINISPAN DEVELOPER


_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev


_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [infinispan-dev] To Optional or not to Optional?

Katia Aresti
Dan, I disagree with point 2 where you say "You now have a field that could be null, Optional.empty(), or Optional.of(something)" 

This is the point of optional. You shouldn't have a field that has these 3 possible values, just two of them = Some or None. If the field is mutable, it should be initialised to Optional.empty(). In the case of an API, Optional implicitly says that the return value can be empty, but when you return a "normal" object, either the user reads the doc, either will have bugs or boilerplate code defending from the possible null value (even if never ever this API will return null)

:o)

Cheers 



On Tue, May 23, 2017 at 3:58 PM, Dan Berindei <[hidden email]> wrote:
I wouldn't say I'm an extreme naysayer, but I do have 2 issues with Optional:

1. Performance becomes harder to quantify: the allocations may or may not be eliminated, and a change in one part of the code may change how allocations are eliminated in a completely different part of the code.
2. My personal opinion is it's just ugly... instead of having one field that could be null or non-null, you now have a field that could be null, Optional.empty(), or Optional.of(something).

Cheers
Dan



On Tue, May 23, 2017 at 1:54 PM, Sebastian Laskawiec <[hidden email]> wrote:
Hey!

So I think we have no extreme naysayers to Optional. So let me try to sum up what we have achieved so:
  • In macroscale benchmark based on REST interface using Optionals didn't lower the performance.
  • +1 for using it in public APIs, especially for those using functional style. 
  • Creating lots of Optional instances might add some pressure on GC, so we need to be careful when using them in hot code paths. In such cases it is required to run a micro scale benchamark to make sure the performance didn't drop. The microbenchmark should also be followed by macro scale benchamrk - PerfJobAck. Also, keep an eye on Eden space in such cases.
If you agree with me, and there are no hard evidence that using Optional degrade performance significantly, I would like to issue a pull request and put those findings into contributing guide [1].

Thanks,
Sebastian


On Mon, May 22, 2017 at 6:36 PM Galder Zamarreño <[hidden email]> wrote:
I think Sanne's right here, any differences in such large scale test are hard to decipher.

Also, as mentioned in a previous email, my view on its usage is same as Sanne's:

* Definitely in APIs/SPIs.
* Be gentle with it internals.

Cheers,
--
Galder Zamarreño
Infinispan, Red Hat

> On 18 May 2017, at 14:35, Sanne Grinovero <[hidden email]> wrote:
>
> Hi Sebastian,
>
> sorry but I think you've been wasting time, I hope it was fun :) This is not the right methodology to "settle" the matter (unless you want Radim's eyes to get bloody..).
>
> Any change in such a complex system will only affect the performance metrics if you're actually addressing the dominant bottleneck. In some cases it might be CPU, like if your system is at 90%+ CPU then it's likely that reviewing the code to use less CPU would be beneficial; but even that can be counter-productive, for example if you're having contention caused by optimistic locking and you fail to address that while making something else "faster" the performance loss on the optimistic lock might become asymptotic.
>
> A good reason to avoid excessive usage of Optional (and *excessive* doesn't mean a couple dozen in a millions lines of code..) is to not run out of eden space, especially for all the code running in interpreted mode.
>
> In your case you've been benchmarking a hugely complex beast, not least over REST! When running the REST Server I doubt that allocation in eden is your main problem. You just happened to have a couple Optionals on your path; sure performance changed but there's no enough data in this way to figure out what exactly happened:
>  - did it change at all or was it just because of a lucky optimisation? (The JIT will always optimise stuff differently even when re-running the same code)
>  - did the overall picture improve because this code became much *less* slower?
>
> The real complexity in benchmarking is to accurately understand why it changed; this should also tell you why it didn't change more, or less..
>
> To be fair I actually agree that it's very likely that C2 can make any performance penalty disappear.. that's totally possible, although it's unlikely to be faster than just reading the field (assuming we don't need to do branching because of null-checks but C2 can optimise that as well).
> Still this requires the code to be optimised by JIT first, so it won't prevent us from creating a gazillion of instances if we abuse its usage irresponsibly. Fighting internal NPEs is a matter of writing better code; I'm not against some "Optional" being strategically placed but I believe it's much nicer for most internal code to just avoid null, use "final", and initialize things aggressively.
>
> Sure use Optional where it makes sense, probably most on APIs and SPIs, but please don't go overboard with it in internals. That's all I said in the original debate.
>
> In case you want to benchmark the impact of Optional make a JMH based microbenchmark - that's interesting to see what C2 is capable of - but even so that's not going to tell you much on the impact it would have to patch thousands of code all around Infinispan. And it will need some peer review before it can tell you anything at all ;)
>
> It's actually a very challenging topic, as we produce libraries meant for "anyone to use" and don't get to set the hardware specification requirements it's hard to predict if we should optimise the system for this/that resource consumption. Some people will have plenty of CPU and have problems with us needing too much memory, some others will have the opposite.. the real challenge is in making internals "elastic" to such factors and adaptable without making it too hard to tune.
>
> Thanks,
> Sanne
>
>
>
> On 18 May 2017 at 12:30, Sebastian Laskawiec <[hidden email]> wrote:
> Hey!
>
> In our past we had a couple of discussions about whether we should or should not use Optionals [1][2]. The main argument against it was performance.
>
> On one hand we risk additional object allocation (the Optional itself) and wrong inlining decisions taken by C2 compiler [3]. On the other hand we all probably "feel" that both of those things shouldn't be a problem and should be optimized by C2. Another argument was the Optional's doesn't give us anything but as I checked, we introduced nearly 80 NullPointerException bugs in two years [4]. So we might consider Optional as a way of fighting those things. The final argument that I've seen was about lack of higher order functions which is simply not true since we have #map, #filter and #flatmap functions. You can do pretty amazing things with this.
>
> I decided to check the performance when refactoring REST interface. I created a PR with Optionals [5], ran performance tests, removed all Optionals and reran tests. You will be surprised by the results [6]:
>
> Test case
> With Optionals [%]    Without Optionals
> Run 1 Run 2   Avg     Run 1   Run 2   Avg
> Non-TX reads 10 threads
> Throughput    32.54   32.87   32.71   31.74   34.04   32.89
> Response time -24.12  -24.63  -24.38  -24.37  -25.69  -25.03
> Non-TX reads 100 threads
> Throughput    6.48    -12.79  -3.16   -7.06   -6.14   -6.60
> Response time -6.15   14.93   4.39    7.88    6.49    7.19
> Non-TX writes 10 threads
> Throughput    9.21    7.60    8.41    4.66    7.15    5.91
> Response time -8.92   -7.11   -8.02   -5.29   -6.93   -6.11
> Non-TX writes 100 threads
> Throughput    2.53    1.65    2.09    -1.16   4.67    1.76
> Response time -2.13   -1.79   -1.96   0.91    -4.67   -1.88
>
> I also created JMH + Flight Recorder tests and again, the results showed no evidence of slow down caused by Optionals [7].
>
> Now please take those results with a grain of salt since they tend to drift by a factor of +/-5% (sometimes even more). But it's very clear the performance results are very similar if not the same.
>
> Having those numbers at hand, do we want to have Optionals in Infinispan codebase or not? And if not, let's state it very clearly (and write it into contributing guide), it's because we don't like them. Not because of performance.
>
> Thanks,
> Sebastian
>
> [1] http://lists.jboss.org/pipermail/infinispan-dev/2017-March/017370.html
> [2] http://lists.jboss.org/pipermail/infinispan-dev/2016-August/016796.html
> [3] http://vanillajava.blogspot.ro/2015/01/java-lambdas-and-low-latency.html
> [4] https://issues.jboss.org/issues/?jql=project%20%3D%20ISPN%20AND%20issuetype%20%3D%20Bug%20AND%20text%20%7E%20%22NullPointerException%22%20AND%20created%20%3E%3D%202015-04-27%20AND%20created%20%3C%3D%202017-04-27
> [5] https://github.com/infinispan/infinispan/pull/5094
> [6] https://docs.google.com/a/redhat.com/spreadsheets/d/1oep6Was0FfvHdqBCwpCFIqcPfJZ5-5_YYUqlRtUxEkM/edit?usp=sharing
> [7] https://github.com/infinispan/infinispan/pull/5094#issuecomment-296970673
> --
> SEBASTIAN ŁASKAWIEC
> INFINISPAN DEVELOPER
> Red Hat EMEA
>
>
> _______________________________________________
> infinispan-dev mailing list
> [hidden email]
> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>
> _______________________________________________
> infinispan-dev mailing list
> [hidden email]
> https://lists.jboss.org/mailman/listinfo/infinispan-dev


_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev
--

SEBASTIAN ŁASKAWIEC

INFINISPAN DEVELOPER


_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev


_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev


_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [infinispan-dev] To Optional or not to Optional?

Vittorio Rigamonti
In reply to this post by Dan Berindei
+1 to Dan's opinion

On Tue, May 23, 2017 at 3:58 PM, Dan Berindei <[hidden email]> wrote:
I wouldn't say I'm an extreme naysayer, but I do have 2 issues with Optional:

1. Performance becomes harder to quantify: the allocations may or may not be eliminated, and a change in one part of the code may change how allocations are eliminated in a completely different part of the code.
2. My personal opinion is it's just ugly... instead of having one field that could be null or non-null, you now have a field that could be null, Optional.empty(), or Optional.of(something).

Cheers
Dan



On Tue, May 23, 2017 at 1:54 PM, Sebastian Laskawiec <[hidden email]> wrote:
Hey!

So I think we have no extreme naysayers to Optional. So let me try to sum up what we have achieved so:
  • In macroscale benchmark based on REST interface using Optionals didn't lower the performance.
  • +1 for using it in public APIs, especially for those using functional style. 
  • Creating lots of Optional instances might add some pressure on GC, so we need to be careful when using them in hot code paths. In such cases it is required to run a micro scale benchamark to make sure the performance didn't drop. The microbenchmark should also be followed by macro scale benchamrk - PerfJobAck. Also, keep an eye on Eden space in such cases.
If you agree with me, and there are no hard evidence that using Optional degrade performance significantly, I would like to issue a pull request and put those findings into contributing guide [1].

Thanks,
Sebastian


On Mon, May 22, 2017 at 6:36 PM Galder Zamarreño <[hidden email]> wrote:
I think Sanne's right here, any differences in such large scale test are hard to decipher.

Also, as mentioned in a previous email, my view on its usage is same as Sanne's:

* Definitely in APIs/SPIs.
* Be gentle with it internals.

Cheers,
--
Galder Zamarreño
Infinispan, Red Hat

> On 18 May 2017, at 14:35, Sanne Grinovero <[hidden email]> wrote:
>
> Hi Sebastian,
>
> sorry but I think you've been wasting time, I hope it was fun :) This is not the right methodology to "settle" the matter (unless you want Radim's eyes to get bloody..).
>
> Any change in such a complex system will only affect the performance metrics if you're actually addressing the dominant bottleneck. In some cases it might be CPU, like if your system is at 90%+ CPU then it's likely that reviewing the code to use less CPU would be beneficial; but even that can be counter-productive, for example if you're having contention caused by optimistic locking and you fail to address that while making something else "faster" the performance loss on the optimistic lock might become asymptotic.
>
> A good reason to avoid excessive usage of Optional (and *excessive* doesn't mean a couple dozen in a millions lines of code..) is to not run out of eden space, especially for all the code running in interpreted mode.
>
> In your case you've been benchmarking a hugely complex beast, not least over REST! When running the REST Server I doubt that allocation in eden is your main problem. You just happened to have a couple Optionals on your path; sure performance changed but there's no enough data in this way to figure out what exactly happened:
>  - did it change at all or was it just because of a lucky optimisation? (The JIT will always optimise stuff differently even when re-running the same code)
>  - did the overall picture improve because this code became much *less* slower?
>
> The real complexity in benchmarking is to accurately understand why it changed; this should also tell you why it didn't change more, or less..
>
> To be fair I actually agree that it's very likely that C2 can make any performance penalty disappear.. that's totally possible, although it's unlikely to be faster than just reading the field (assuming we don't need to do branching because of null-checks but C2 can optimise that as well).
> Still this requires the code to be optimised by JIT first, so it won't prevent us from creating a gazillion of instances if we abuse its usage irresponsibly. Fighting internal NPEs is a matter of writing better code; I'm not against some "Optional" being strategically placed but I believe it's much nicer for most internal code to just avoid null, use "final", and initialize things aggressively.
>
> Sure use Optional where it makes sense, probably most on APIs and SPIs, but please don't go overboard with it in internals. That's all I said in the original debate.
>
> In case you want to benchmark the impact of Optional make a JMH based microbenchmark - that's interesting to see what C2 is capable of - but even so that's not going to tell you much on the impact it would have to patch thousands of code all around Infinispan. And it will need some peer review before it can tell you anything at all ;)
>
> It's actually a very challenging topic, as we produce libraries meant for "anyone to use" and don't get to set the hardware specification requirements it's hard to predict if we should optimise the system for this/that resource consumption. Some people will have plenty of CPU and have problems with us needing too much memory, some others will have the opposite.. the real challenge is in making internals "elastic" to such factors and adaptable without making it too hard to tune.
>
> Thanks,
> Sanne
>
>
>
> On 18 May 2017 at 12:30, Sebastian Laskawiec <[hidden email]> wrote:
> Hey!
>
> In our past we had a couple of discussions about whether we should or should not use Optionals [1][2]. The main argument against it was performance.
>
> On one hand we risk additional object allocation (the Optional itself) and wrong inlining decisions taken by C2 compiler [3]. On the other hand we all probably "feel" that both of those things shouldn't be a problem and should be optimized by C2. Another argument was the Optional's doesn't give us anything but as I checked, we introduced nearly 80 NullPointerException bugs in two years [4]. So we might consider Optional as a way of fighting those things. The final argument that I've seen was about lack of higher order functions which is simply not true since we have #map, #filter and #flatmap functions. You can do pretty amazing things with this.
>
> I decided to check the performance when refactoring REST interface. I created a PR with Optionals [5], ran performance tests, removed all Optionals and reran tests. You will be surprised by the results [6]:
>
> Test case
> With Optionals [%]    Without Optionals
> Run 1 Run 2   Avg     Run 1   Run 2   Avg
> Non-TX reads 10 threads
> Throughput    32.54   32.87   32.71   31.74   34.04   32.89
> Response time -24.12  -24.63  -24.38  -24.37  -25.69  -25.03
> Non-TX reads 100 threads
> Throughput    6.48    -12.79  -3.16   -7.06   -6.14   -6.60
> Response time -6.15   14.93   4.39    7.88    6.49    7.19
> Non-TX writes 10 threads
> Throughput    9.21    7.60    8.41    4.66    7.15    5.91
> Response time -8.92   -7.11   -8.02   -5.29   -6.93   -6.11
> Non-TX writes 100 threads
> Throughput    2.53    1.65    2.09    -1.16   4.67    1.76
> Response time -2.13   -1.79   -1.96   0.91    -4.67   -1.88
>
> I also created JMH + Flight Recorder tests and again, the results showed no evidence of slow down caused by Optionals [7].
>
> Now please take those results with a grain of salt since they tend to drift by a factor of +/-5% (sometimes even more). But it's very clear the performance results are very similar if not the same.
>
> Having those numbers at hand, do we want to have Optionals in Infinispan codebase or not? And if not, let's state it very clearly (and write it into contributing guide), it's because we don't like them. Not because of performance.
>
> Thanks,
> Sebastian
>
> [1] http://lists.jboss.org/pipermail/infinispan-dev/2017-March/017370.html
> [2] http://lists.jboss.org/pipermail/infinispan-dev/2016-August/016796.html
> [3] http://vanillajava.blogspot.ro/2015/01/java-lambdas-and-low-latency.html
> [4] https://issues.jboss.org/issues/?jql=project%20%3D%20ISPN%20AND%20issuetype%20%3D%20Bug%20AND%20text%20%7E%20%22NullPointerException%22%20AND%20created%20%3E%3D%202015-04-27%20AND%20created%20%3C%3D%202017-04-27
> [5] https://github.com/infinispan/infinispan/pull/5094
> [6] https://docs.google.com/a/redhat.com/spreadsheets/d/1oep6Was0FfvHdqBCwpCFIqcPfJZ5-5_YYUqlRtUxEkM/edit?usp=sharing
> [7] https://github.com/infinispan/infinispan/pull/5094#issuecomment-296970673
> --
> SEBASTIAN ŁASKAWIEC
> INFINISPAN DEVELOPER
> Red Hat EMEA
>
>
> _______________________________________________
> infinispan-dev mailing list
> [hidden email]
> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>
> _______________________________________________
> infinispan-dev mailing list
> [hidden email]
> https://lists.jboss.org/mailman/listinfo/infinispan-dev


_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev
--

SEBASTIAN ŁASKAWIEC

INFINISPAN DEVELOPER


_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev


_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev



--

Vittorio Rigamonti

Senior Software Engineer

Red Hat

Milan, Italy

[hidden email]

irc: rigazilla 


_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [infinispan-dev] To Optional or not to Optional?

Bela Ban-2
In reply to this post by Sebastian Laskawiec
Actually, I'm an extreme naysayer! I actually voiced concerns so I'm
wondering where your assumption there are no naysayers is coming from... :-)


On 23/05/17 1:54 PM, Sebastian Laskawiec wrote:

> Hey!
>
> So I think we have no extreme naysayers to Optional. So let me try to
> sum up what we have achieved so:
>
>   * In macroscale benchmark based on REST interface using Optionals
>     didn't lower the performance.
>   * +1 for using it in public APIs, especially for those using
>     functional style.
>   * Creating lots of Optional instances might add some pressure on GC,
>     so we need to be careful when using them in hot code paths. In
>     such cases it is required to run a micro scale benchamark to make
>     sure the performance didn't drop. The microbenchmark should also
>     be followed by macro scale benchamrk - PerfJobAck. Also, keep an
>     eye on Eden space in such cases.
>
> If you agree with me, and there are no hard evidence that using
> Optional degrade performance significantly, I would like to issue a
> pull request and put those findings into contributing guide [1].
>
> Thanks,
> Sebastian
>
> [1]
> https://github.com/infinispan/infinispan/tree/master/documentation/src/main/asciidoc/contributing
>
> On Mon, May 22, 2017 at 6:36 PM Galder Zamarreño <[hidden email]
> <mailto:[hidden email]>> wrote:
>
>     I think Sanne's right here, any differences in such large scale
>     test are hard to decipher.
>
>     Also, as mentioned in a previous email, my view on its usage is
>     same as Sanne's:
>
>     * Definitely in APIs/SPIs.
>     * Be gentle with it internals.
>
>     Cheers,
>     --
>     Galder Zamarreño
>     Infinispan, Red Hat
>
>     > On 18 May 2017, at 14:35, Sanne Grinovero <[hidden email]
>     <mailto:[hidden email]>> wrote:
>     >
>     > Hi Sebastian,
>     >
>     > sorry but I think you've been wasting time, I hope it was fun :)
>     This is not the right methodology to "settle" the matter (unless
>     you want Radim's eyes to get bloody..).
>     >
>     > Any change in such a complex system will only affect the
>     performance metrics if you're actually addressing the dominant
>     bottleneck. In some cases it might be CPU, like if your system is
>     at 90%+ CPU then it's likely that reviewing the code to use less
>     CPU would be beneficial; but even that can be counter-productive,
>     for example if you're having contention caused by optimistic
>     locking and you fail to address that while making something else
>     "faster" the performance loss on the optimistic lock might become
>     asymptotic.
>     >
>     > A good reason to avoid excessive usage of Optional (and
>     *excessive* doesn't mean a couple dozen in a millions lines of
>     code..) is to not run out of eden space, especially for all the
>     code running in interpreted mode.
>     >
>     > In your case you've been benchmarking a hugely complex beast,
>     not least over REST! When running the REST Server I doubt that
>     allocation in eden is your main problem. You just happened to have
>     a couple Optionals on your path; sure performance changed but
>     there's no enough data in this way to figure out what exactly
>     happened:
>     >  - did it change at all or was it just because of a lucky
>     optimisation? (The JIT will always optimise stuff differently even
>     when re-running the same code)
>     >  - did the overall picture improve because this code became much
>     *less* slower?
>     >
>     > The real complexity in benchmarking is to accurately understand
>     why it changed; this should also tell you why it didn't change
>     more, or less..
>     >
>     > To be fair I actually agree that it's very likely that C2 can
>     make any performance penalty disappear.. that's totally possible,
>     although it's unlikely to be faster than just reading the field
>     (assuming we don't need to do branching because of null-checks but
>     C2 can optimise that as well).
>     > Still this requires the code to be optimised by JIT first, so it
>     won't prevent us from creating a gazillion of instances if we
>     abuse its usage irresponsibly. Fighting internal NPEs is a matter
>     of writing better code; I'm not against some "Optional" being
>     strategically placed but I believe it's much nicer for most
>     internal code to just avoid null, use "final", and initialize
>     things aggressively.
>     >
>     > Sure use Optional where it makes sense, probably most on APIs
>     and SPIs, but please don't go overboard with it in internals.
>     That's all I said in the original debate.
>     >
>     > In case you want to benchmark the impact of Optional make a JMH
>     based microbenchmark - that's interesting to see what C2 is
>     capable of - but even so that's not going to tell you much on the
>     impact it would have to patch thousands of code all around
>     Infinispan. And it will need some peer review before it can tell
>     you anything at all ;)
>     >
>     > It's actually a very challenging topic, as we produce libraries
>     meant for "anyone to use" and don't get to set the hardware
>     specification requirements it's hard to predict if we should
>     optimise the system for this/that resource consumption. Some
>     people will have plenty of CPU and have problems with us needing
>     too much memory, some others will have the opposite.. the real
>     challenge is in making internals "elastic" to such factors and
>     adaptable without making it too hard to tune.
>     >
>     > Thanks,
>     > Sanne
>     >
>     >
>     >
>     > On 18 May 2017 at 12:30, Sebastian Laskawiec
>     <[hidden email] <mailto:[hidden email]>> wrote:
>     > Hey!
>     >
>     > In our past we had a couple of discussions about whether we
>     should or should not use Optionals [1][2]. The main argument
>     against it was performance.
>     >
>     > On one hand we risk additional object allocation (the Optional
>     itself) and wrong inlining decisions taken by C2 compiler [3]. On
>     the other hand we all probably "feel" that both of those things
>     shouldn't be a problem and should be optimized by C2. Another
>     argument was the Optional's doesn't give us anything but as I
>     checked, we introduced nearly 80 NullPointerException bugs in two
>     years [4]. So we might consider Optional as a way of fighting
>     those things. The final argument that I've seen was about lack of
>     higher order functions which is simply not true since we have
>     #map, #filter and #flatmap functions. You can do pretty amazing
>     things with this.
>     >
>     > I decided to check the performance when refactoring REST
>     interface. I created a PR with Optionals [5], ran performance
>     tests, removed all Optionals and reran tests. You will be
>     surprised by the results [6]:
>     >
>     > Test case
>     > With Optionals [%]    Without Optionals
>     > Run 1 Run 2   Avg     Run 1   Run 2   Avg
>     > Non-TX reads 10 threads
>     > Throughput    32.54   32.87   32.71   31.74   34.04  32.89
>     > Response time -24.12  -24.63  -24.38  -24.37  -25.69 -25.03
>     > Non-TX reads 100 threads
>     > Throughput    6.48    -12.79  -3.16   -7.06   -6.14  -6.60
>     > Response time -6.15   14.93   4.39    7.88    6.49 7.19
>     > Non-TX writes 10 threads
>     > Throughput    9.21    7.60    8.41    4.66    7.15 5.91
>     > Response time -8.92   -7.11   -8.02   -5.29   -6.93  -6.11
>     > Non-TX writes 100 threads
>     > Throughput    2.53    1.65    2.09    -1.16   4.67 1.76
>     > Response time -2.13   -1.79   -1.96   0.91    -4.67  -1.88
>     >
>     > I also created JMH + Flight Recorder tests and again, the
>     results showed no evidence of slow down caused by Optionals [7].
>     >
>     > Now please take those results with a grain of salt since they
>     tend to drift by a factor of +/-5% (sometimes even more). But it's
>     very clear the performance results are very similar if not the same.
>     >
>     > Having those numbers at hand, do we want to have Optionals in
>     Infinispan codebase or not? And if not, let's state it very
>     clearly (and write it into contributing guide), it's because we
>     don't like them. Not because of performance.
>     >
>     > Thanks,
>     > Sebastian
>     >
>     > [1]
>     http://lists.jboss.org/pipermail/infinispan-dev/2017-March/017370.html
>     > [2]
>     http://lists.jboss.org/pipermail/infinispan-dev/2016-August/016796.html
>     > [3]
>     http://vanillajava.blogspot.ro/2015/01/java-lambdas-and-low-latency.html
>     > [4]
>     https://issues.jboss.org/issues/?jql=project%20%3D%20ISPN%20AND%20issuetype%20%3D%20Bug%20AND%20text%20%7E%20%22NullPointerException%22%20AND%20created%20%3E%3D%202015-04-27%20AND%20created%20%3C%3D%202017-04-27
>     > [5] https://github.com/infinispan/infinispan/pull/5094
>     > [6]
>     https://docs.google.com/a/redhat.com/spreadsheets/d/1oep6Was0FfvHdqBCwpCFIqcPfJZ5-5_YYUqlRtUxEkM/edit?usp=sharing
>     > [7]
>     https://github.com/infinispan/infinispan/pull/5094#issuecomment-296970673
>     > --
>     > SEBASTIAN ŁASKAWIEC
>     > INFINISPAN DEVELOPER
>     > Red Hat EMEA
>     >
>     >
>     > _______________________________________________
>     > infinispan-dev mailing list
>     > [hidden email]
>     <mailto:[hidden email]>
>     > https://lists.jboss.org/mailman/listinfo/infinispan-dev
>     >
>     > _______________________________________________
>     > infinispan-dev mailing list
>     > [hidden email]
>     <mailto:[hidden email]>
>     > https://lists.jboss.org/mailman/listinfo/infinispan-dev
>
>
>     _______________________________________________
>     infinispan-dev mailing list
>     [hidden email] <mailto:[hidden email]>
>     https://lists.jboss.org/mailman/listinfo/infinispan-dev
>
> --
>
> SEBASTIANŁASKAWIEC
>
> INFINISPAN DEVELOPER
>
> Red HatEMEA <https://www.redhat.com/>
>
> <https://red.ht/sig>
>
>
>
> _______________________________________________
> infinispan-dev mailing list
> [hidden email]
> https://lists.jboss.org/mailman/listinfo/infinispan-dev

--
Bela Ban, JGroups lead (http://www.jgroups.org)

_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [infinispan-dev] To Optional or not to Optional?

Radim Vansa
In reply to this post by Katia Aresti
I haven't checked Sebastian's refactored code, but does it use Optionals
as a *field* type? That's misuse (same as using it as an arg), it's
intended solely as method return type.

Radim

On 05/23/2017 05:45 PM, Katia Aresti wrote:

> Dan, I disagree with point 2 where you say "You now have a field that
> could be null, Optional.empty(), or Optional.of(something)"
>
> This is the point of optional. You shouldn't have a field that has
> these 3 possible values, just two of them = Some or None. If the field
> is mutable, it should be initialised to Optional.empty(). In the case
> of an API, Optional implicitly says that the return value can be
> empty, but when you return a "normal" object, either the user reads
> the doc, either will have bugs or boilerplate code defending from the
> possible null value (even if never ever this API will return null)
>
> :o)
>
> Cheers
>
>
>
> On Tue, May 23, 2017 at 3:58 PM, Dan Berindei <[hidden email]
> <mailto:[hidden email]>> wrote:
>
>     I wouldn't say I'm an extreme naysayer, but I do have 2 issues
>     with Optional:
>
>     1. Performance becomes harder to quantify: the allocations may or
>     may not be eliminated, and a change in one part of the code may
>     change how allocations are eliminated in a completely different
>     part of the code.
>     2. My personal opinion is it's just ugly... instead of having one
>     field that could be null or non-null, you now have a field that
>     could be null, Optional.empty(), or Optional.of(something).
>
>     Cheers
>     Dan
>
>
>
>     On Tue, May 23, 2017 at 1:54 PM, Sebastian Laskawiec
>     <[hidden email] <mailto:[hidden email]>> wrote:
>
>         Hey!
>
>         So I think we have no extreme naysayers to Optional. So let me
>         try to sum up what we have achieved so:
>
>           * In macroscale benchmark based on REST interface using
>             Optionals didn't lower the performance.
>           * +1 for using it in public APIs, especially for those using
>             functional style.
>           * Creating lots of Optional instances might add some
>             pressure on GC, so we need to be careful when using them
>             in hot code paths. In such cases it is required to run a
>             micro scale benchamark to make sure the performance didn't
>             drop. The microbenchmark should also be followed by macro
>             scale benchamrk - PerfJobAck. Also, keep an eye on Eden
>             space in such cases.
>
>         If you agree with me, and there are no hard evidence that
>         using Optional degrade performance significantly, I would like
>         to issue a pull request and put those findings into
>         contributing guide [1].
>
>         Thanks,
>         Sebastian
>
>         [1]
>         https://github.com/infinispan/infinispan/tree/master/documentation/src/main/asciidoc/contributing
>         <https://github.com/infinispan/infinispan/tree/master/documentation/src/main/asciidoc/contributing>
>
>         On Mon, May 22, 2017 at 6:36 PM Galder Zamarreño
>         <[hidden email] <mailto:[hidden email]>> wrote:
>
>             I think Sanne's right here, any differences in such large
>             scale test are hard to decipher.
>
>             Also, as mentioned in a previous email, my view on its
>             usage is same as Sanne's:
>
>             * Definitely in APIs/SPIs.
>             * Be gentle with it internals.
>
>             Cheers,
>             --
>             Galder Zamarreño
>             Infinispan, Red Hat
>
>             > On 18 May 2017, at 14:35, Sanne Grinovero
>             <[hidden email] <mailto:[hidden email]>> wrote:
>             >
>             > Hi Sebastian,
>             >
>             > sorry but I think you've been wasting time, I hope it
>             was fun :) This is not the right methodology to "settle"
>             the matter (unless you want Radim's eyes to get bloody..).
>             >
>             > Any change in such a complex system will only affect the
>             performance metrics if you're actually addressing the
>             dominant bottleneck. In some cases it might be CPU, like
>             if your system is at 90%+ CPU then it's likely that
>             reviewing the code to use less CPU would be beneficial;
>             but even that can be counter-productive, for example if
>             you're having contention caused by optimistic locking and
>             you fail to address that while making something else
>             "faster" the performance loss on the optimistic lock might
>             become asymptotic.
>             >
>             > A good reason to avoid excessive usage of Optional (and
>             *excessive* doesn't mean a couple dozen in a millions
>             lines of code..) is to not run out of eden space,
>             especially for all the code running in interpreted mode.
>             >
>             > In your case you've been benchmarking a hugely complex
>             beast, not least over REST! When running the REST Server I
>             doubt that allocation in eden is your main problem. You
>             just happened to have a couple Optionals on your path;
>             sure performance changed but there's no enough data in
>             this way to figure out what exactly happened:
>             >  - did it change at all or was it just because of a
>             lucky optimisation? (The JIT will always optimise stuff
>             differently even when re-running the same code)
>             >  - did the overall picture improve because this code
>             became much *less* slower?
>             >
>             > The real complexity in benchmarking is to accurately
>             understand why it changed; this should also tell you why
>             it didn't change more, or less..
>             >
>             > To be fair I actually agree that it's very likely that
>             C2 can make any performance penalty disappear.. that's
>             totally possible, although it's unlikely to be faster than
>             just reading the field (assuming we don't need to do
>             branching because of null-checks but C2 can optimise that
>             as well).
>             > Still this requires the code to be optimised by JIT
>             first, so it won't prevent us from creating a gazillion of
>             instances if we abuse its usage irresponsibly. Fighting
>             internal NPEs is a matter of writing better code; I'm not
>             against some "Optional" being strategically placed but I
>             believe it's much nicer for most internal code to just
>             avoid null, use "final", and initialize things aggressively.
>             >
>             > Sure use Optional where it makes sense, probably most on
>             APIs and SPIs, but please don't go overboard with it in
>             internals. That's all I said in the original debate.
>             >
>             > In case you want to benchmark the impact of Optional
>             make a JMH based microbenchmark - that's interesting to
>             see what C2 is capable of - but even so that's not going
>             to tell you much on the impact it would have to patch
>             thousands of code all around Infinispan. And it will need
>             some peer review before it can tell you anything at all ;)
>             >
>             > It's actually a very challenging topic, as we produce
>             libraries meant for "anyone to use" and don't get to set
>             the hardware specification requirements it's hard to
>             predict if we should optimise the system for this/that
>             resource consumption. Some people will have plenty of CPU
>             and have problems with us needing too much memory, some
>             others will have the opposite.. the real challenge is in
>             making internals "elastic" to such factors and adaptable
>             without making it too hard to tune.
>             >
>             > Thanks,
>             > Sanne
>             >
>             >
>             >
>             > On 18 May 2017 at 12:30, Sebastian Laskawiec
>             <[hidden email] <mailto:[hidden email]>> wrote:
>             > Hey!
>             >
>             > In our past we had a couple of discussions about whether
>             we should or should not use Optionals [1][2]. The main
>             argument against it was performance.
>             >
>             > On one hand we risk additional object allocation (the
>             Optional itself) and wrong inlining decisions taken by C2
>             compiler [3]. On the other hand we all probably "feel"
>             that both of those things shouldn't be a problem and
>             should be optimized by C2. Another argument was the
>             Optional's doesn't give us anything but as I checked, we
>             introduced nearly 80 NullPointerException bugs in two
>             years [4]. So we might consider Optional as a way of
>             fighting those things. The final argument that I've seen
>             was about lack of higher order functions which is simply
>             not true since we have #map, #filter and #flatmap
>             functions. You can do pretty amazing things with this.
>             >
>             > I decided to check the performance when refactoring REST
>             interface. I created a PR with Optionals [5], ran
>             performance tests, removed all Optionals and reran tests.
>             You will be surprised by the results [6]:
>             >
>             > Test case
>             > With Optionals [%]    Without Optionals
>             > Run 1 Run 2   Avg     Run 1   Run 2   Avg
>             > Non-TX reads 10 threads
>             > Throughput    32.54   32.87  32.71   31.74   34.04   32.89
>             > Response time -24.12  -24.63 -24.38  -24.37  -25.69  -25.03
>             > Non-TX reads 100 threads
>             > Throughput    6.48    -12.79 -3.16   -7.06   -6.14   -6.60
>             > Response time -6.15   14.93   4.39   7.88    6.49    7.19
>             > Non-TX writes 10 threads
>             > Throughput    9.21    7.60    8.41   4.66    7.15    5.91
>             > Response time -8.92   -7.11  -8.02   -5.29   -6.93   -6.11
>             > Non-TX writes 100 threads
>             > Throughput    2.53    1.65    2.09   -1.16   4.67    1.76
>             > Response time -2.13   -1.79  -1.96   0.91    -4.67   -1.88
>             >
>             > I also created JMH + Flight Recorder tests and again,
>             the results showed no evidence of slow down caused by
>             Optionals [7].
>             >
>             > Now please take those results with a grain of salt since
>             they tend to drift by a factor of +/-5% (sometimes even
>             more). But it's very clear the performance results are
>             very similar if not the same.
>             >
>             > Having those numbers at hand, do we want to have
>             Optionals in Infinispan codebase or not? And if not, let's
>             state it very clearly (and write it into contributing
>             guide), it's because we don't like them. Not because of
>             performance.
>             >
>             > Thanks,
>             > Sebastian
>             >
>             > [1]
>             http://lists.jboss.org/pipermail/infinispan-dev/2017-March/017370.html
>             <http://lists.jboss.org/pipermail/infinispan-dev/2017-March/017370.html>
>             > [2]
>             http://lists.jboss.org/pipermail/infinispan-dev/2016-August/016796.html
>             <http://lists.jboss.org/pipermail/infinispan-dev/2016-August/016796.html>
>             > [3]
>             http://vanillajava.blogspot.ro/2015/01/java-lambdas-and-low-latency.html
>             <http://vanillajava.blogspot.ro/2015/01/java-lambdas-and-low-latency.html>
>             > [4]
>             https://issues.jboss.org/issues/?jql=project%20%3D%20ISPN%20AND%20issuetype%20%3D%20Bug%20AND%20text%20%7E%20%22NullPointerException%22%20AND%20created%20%3E%3D%202015-04-27%20AND%20created%20%3C%3D%202017-04-27
>             <https://issues.jboss.org/issues/?jql=project%20%3D%20ISPN%20AND%20issuetype%20%3D%20Bug%20AND%20text%20%7E%20%22NullPointerException%22%20AND%20created%20%3E%3D%202015-04-27%20AND%20created%20%3C%3D%202017-04-27>
>             > [5] https://github.com/infinispan/infinispan/pull/5094
>             <https://github.com/infinispan/infinispan/pull/5094>
>             > [6]
>             https://docs.google.com/a/redhat.com/spreadsheets/d/1oep6Was0FfvHdqBCwpCFIqcPfJZ5-5_YYUqlRtUxEkM/edit?usp=sharing
>             <https://docs.google.com/a/redhat.com/spreadsheets/d/1oep6Was0FfvHdqBCwpCFIqcPfJZ5-5_YYUqlRtUxEkM/edit?usp=sharing>
>             > [7]
>             https://github.com/infinispan/infinispan/pull/5094#issuecomment-296970673
>             <https://github.com/infinispan/infinispan/pull/5094#issuecomment-296970673>
>             > --
>             > SEBASTIAN ŁASKAWIEC
>             > INFINISPAN DEVELOPER
>             > Red Hat EMEA
>             >
>             >
>             > _______________________________________________
>             > infinispan-dev mailing list
>             > [hidden email]
>             <mailto:[hidden email]>
>             > https://lists.jboss.org/mailman/listinfo/infinispan-dev
>             <https://lists.jboss.org/mailman/listinfo/infinispan-dev>
>             >
>             > _______________________________________________
>             > infinispan-dev mailing list
>             > [hidden email]
>             <mailto:[hidden email]>
>             > https://lists.jboss.org/mailman/listinfo/infinispan-dev
>             <https://lists.jboss.org/mailman/listinfo/infinispan-dev>
>
>
>             _______________________________________________
>             infinispan-dev mailing list
>             [hidden email]
>             <mailto:[hidden email]>
>             https://lists.jboss.org/mailman/listinfo/infinispan-dev
>             <https://lists.jboss.org/mailman/listinfo/infinispan-dev>
>
>         --
>
>         SEBASTIANŁASKAWIEC
>
>         INFINISPAN DEVELOPER
>
>         Red HatEMEA <https://www.redhat.com/>
>
>         <https://red.ht/sig>
>
>
>         _______________________________________________
>         infinispan-dev mailing list
>         [hidden email]
>         <mailto:[hidden email]>
>         https://lists.jboss.org/mailman/listinfo/infinispan-dev
>         <https://lists.jboss.org/mailman/listinfo/infinispan-dev>
>
>
>
>     _______________________________________________
>     infinispan-dev mailing list
>     [hidden email] <mailto:[hidden email]>
>     https://lists.jboss.org/mailman/listinfo/infinispan-dev
>     <https://lists.jboss.org/mailman/listinfo/infinispan-dev>
>
>
>
>
> _______________________________________________
> infinispan-dev mailing list
> [hidden email]
> https://lists.jboss.org/mailman/listinfo/infinispan-dev


--
Radim Vansa <[hidden email]>
JBoss Performance Team

_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [infinispan-dev] To Optional or not to Optional?

Sebastian Laskawiec
In reply to this post by Bela Ban-2
Indeed Bela, you're an extreme naysayer! :)

I'm actually trying to get as many comments and arguments out of this discussion. I hope we will be able to iron out a general recommendation or approach how we want to treat Optionals.

On Tue, May 23, 2017 at 10:14 PM Bela Ban <[hidden email]> wrote:
Actually, I'm an extreme naysayer! I actually voiced concerns so I'm
wondering where your assumption there are no naysayers is coming from... :-)


On 23/05/17 1:54 PM, Sebastian Laskawiec wrote:
> Hey!
>
> So I think we have no extreme naysayers to Optional. So let me try to
> sum up what we have achieved so:
>
>   * In macroscale benchmark based on REST interface using Optionals
>     didn't lower the performance.
>   * +1 for using it in public APIs, especially for those using
>     functional style.
>   * Creating lots of Optional instances might add some pressure on GC,
>     so we need to be careful when using them in hot code paths. In
>     such cases it is required to run a micro scale benchamark to make
>     sure the performance didn't drop. The microbenchmark should also
>     be followed by macro scale benchamrk - PerfJobAck. Also, keep an
>     eye on Eden space in such cases.
>
> If you agree with me, and there are no hard evidence that using
> Optional degrade performance significantly, I would like to issue a
> pull request and put those findings into contributing guide [1].
>
> Thanks,
> Sebastian
>
> [1]
> https://github.com/infinispan/infinispan/tree/master/documentation/src/main/asciidoc/contributing
>
> On Mon, May 22, 2017 at 6:36 PM Galder Zamarreño <[hidden email]
> <mailto:[hidden email]>> wrote:
>
>     I think Sanne's right here, any differences in such large scale
>     test are hard to decipher.
>
>     Also, as mentioned in a previous email, my view on its usage is
>     same as Sanne's:
>
>     * Definitely in APIs/SPIs.
>     * Be gentle with it internals.
>
>     Cheers,
>     --
>     Galder Zamarreño
>     Infinispan, Red Hat
>
>     > On 18 May 2017, at 14:35, Sanne Grinovero <[hidden email]
>     <mailto:[hidden email]>> wrote:
>     >
>     > Hi Sebastian,
>     >
>     > sorry but I think you've been wasting time, I hope it was fun :)
>     This is not the right methodology to "settle" the matter (unless
>     you want Radim's eyes to get bloody..).
>     >
>     > Any change in such a complex system will only affect the
>     performance metrics if you're actually addressing the dominant
>     bottleneck. In some cases it might be CPU, like if your system is
>     at 90%+ CPU then it's likely that reviewing the code to use less
>     CPU would be beneficial; but even that can be counter-productive,
>     for example if you're having contention caused by optimistic
>     locking and you fail to address that while making something else
>     "faster" the performance loss on the optimistic lock might become
>     asymptotic.
>     >
>     > A good reason to avoid excessive usage of Optional (and
>     *excessive* doesn't mean a couple dozen in a millions lines of
>     code..) is to not run out of eden space, especially for all the
>     code running in interpreted mode.
>     >
>     > In your case you've been benchmarking a hugely complex beast,
>     not least over REST! When running the REST Server I doubt that
>     allocation in eden is your main problem. You just happened to have
>     a couple Optionals on your path; sure performance changed but
>     there's no enough data in this way to figure out what exactly
>     happened:
>     >  - did it change at all or was it just because of a lucky
>     optimisation? (The JIT will always optimise stuff differently even
>     when re-running the same code)
>     >  - did the overall picture improve because this code became much
>     *less* slower?
>     >
>     > The real complexity in benchmarking is to accurately understand
>     why it changed; this should also tell you why it didn't change
>     more, or less..
>     >
>     > To be fair I actually agree that it's very likely that C2 can
>     make any performance penalty disappear.. that's totally possible,
>     although it's unlikely to be faster than just reading the field
>     (assuming we don't need to do branching because of null-checks but
>     C2 can optimise that as well).
>     > Still this requires the code to be optimised by JIT first, so it
>     won't prevent us from creating a gazillion of instances if we
>     abuse its usage irresponsibly. Fighting internal NPEs is a matter
>     of writing better code; I'm not against some "Optional" being
>     strategically placed but I believe it's much nicer for most
>     internal code to just avoid null, use "final", and initialize
>     things aggressively.
>     >
>     > Sure use Optional where it makes sense, probably most on APIs
>     and SPIs, but please don't go overboard with it in internals.
>     That's all I said in the original debate.
>     >
>     > In case you want to benchmark the impact of Optional make a JMH
>     based microbenchmark - that's interesting to see what C2 is
>     capable of - but even so that's not going to tell you much on the
>     impact it would have to patch thousands of code all around
>     Infinispan. And it will need some peer review before it can tell
>     you anything at all ;)
>     >
>     > It's actually a very challenging topic, as we produce libraries
>     meant for "anyone to use" and don't get to set the hardware
>     specification requirements it's hard to predict if we should
>     optimise the system for this/that resource consumption. Some
>     people will have plenty of CPU and have problems with us needing
>     too much memory, some others will have the opposite.. the real
>     challenge is in making internals "elastic" to such factors and
>     adaptable without making it too hard to tune.
>     >
>     > Thanks,
>     > Sanne
>     >
>     >
>     >
>     > On 18 May 2017 at 12:30, Sebastian Laskawiec
>     <[hidden email] <mailto:[hidden email]>> wrote:
>     > Hey!
>     >
>     > In our past we had a couple of discussions about whether we
>     should or should not use Optionals [1][2]. The main argument
>     against it was performance.
>     >
>     > On one hand we risk additional object allocation (the Optional
>     itself) and wrong inlining decisions taken by C2 compiler [3]. On
>     the other hand we all probably "feel" that both of those things
>     shouldn't be a problem and should be optimized by C2. Another
>     argument was the Optional's doesn't give us anything but as I
>     checked, we introduced nearly 80 NullPointerException bugs in two
>     years [4]. So we might consider Optional as a way of fighting
>     those things. The final argument that I've seen was about lack of
>     higher order functions which is simply not true since we have
>     #map, #filter and #flatmap functions. You can do pretty amazing
>     things with this.
>     >
>     > I decided to check the performance when refactoring REST
>     interface. I created a PR with Optionals [5], ran performance
>     tests, removed all Optionals and reran tests. You will be
>     surprised by the results [6]:
>     >
>     > Test case
>     > With Optionals [%]    Without Optionals
>     > Run 1 Run 2   Avg     Run 1   Run 2   Avg
>     > Non-TX reads 10 threads
>     > Throughput    32.54   32.87   32.71   31.74   34.04  32.89
>     > Response time -24.12  -24.63  -24.38  -24.37  -25.69 -25.03
>     > Non-TX reads 100 threads
>     > Throughput    6.48    -12.79  -3.16   -7.06   -6.14  -6.60
>     > Response time -6.15   14.93   4.39    7.88    6.49 7.19
>     > Non-TX writes 10 threads
>     > Throughput    9.21    7.60    8.41    4.66    7.15 5.91
>     > Response time -8.92   -7.11   -8.02   -5.29   -6.93  -6.11
>     > Non-TX writes 100 threads
>     > Throughput    2.53    1.65    2.09    -1.16   4.67 1.76
>     > Response time -2.13   -1.79   -1.96   0.91    -4.67  -1.88
>     >
>     > I also created JMH + Flight Recorder tests and again, the
>     results showed no evidence of slow down caused by Optionals [7].
>     >
>     > Now please take those results with a grain of salt since they
>     tend to drift by a factor of +/-5% (sometimes even more). But it's
>     very clear the performance results are very similar if not the same.
>     >
>     > Having those numbers at hand, do we want to have Optionals in
>     Infinispan codebase or not? And if not, let's state it very
>     clearly (and write it into contributing guide), it's because we
>     don't like them. Not because of performance.
>     >
>     > Thanks,
>     > Sebastian
>     >
>     > [1]
>     http://lists.jboss.org/pipermail/infinispan-dev/2017-March/017370.html
>     > [2]
>     http://lists.jboss.org/pipermail/infinispan-dev/2016-August/016796.html
>     > [3]
>     http://vanillajava.blogspot.ro/2015/01/java-lambdas-and-low-latency.html
>     > [4]
>     https://issues.jboss.org/issues/?jql=project%20%3D%20ISPN%20AND%20issuetype%20%3D%20Bug%20AND%20text%20%7E%20%22NullPointerException%22%20AND%20created%20%3E%3D%202015-04-27%20AND%20created%20%3C%3D%202017-04-27
>     > [5] https://github.com/infinispan/infinispan/pull/5094
>     > [6]
>     https://docs.google.com/a/redhat.com/spreadsheets/d/1oep6Was0FfvHdqBCwpCFIqcPfJZ5-5_YYUqlRtUxEkM/edit?usp=sharing
>     > [7]
>     https://github.com/infinispan/infinispan/pull/5094#issuecomment-296970673
>     > --
>     > SEBASTIAN ŁASKAWIEC
>     > INFINISPAN DEVELOPER
>     > Red Hat EMEA
>     >
>     >
>     > _______________________________________________
>     > infinispan-dev mailing list
>     > [hidden email]
>     <mailto:[hidden email]>
>     > https://lists.jboss.org/mailman/listinfo/infinispan-dev
>     >
>     > _______________________________________________
>     > infinispan-dev mailing list
>     > [hidden email]
>     <mailto:[hidden email]>
>     > https://lists.jboss.org/mailman/listinfo/infinispan-dev
>
>
>     _______________________________________________
>     infinispan-dev mailing list
>     [hidden email] <mailto:[hidden email]>
>     https://lists.jboss.org/mailman/listinfo/infinispan-dev
>
> --
>
> SEBASTIANŁASKAWIEC
>
> INFINISPAN DEVELOPER
>
> Red HatEMEA <https://www.redhat.com/>
>
> <https://red.ht/sig>
>
>
>
> _______________________________________________
> infinispan-dev mailing list
> [hidden email]
> https://lists.jboss.org/mailman/listinfo/infinispan-dev

--
Bela Ban, JGroups lead (http://www.jgroups.org)

_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev
--

SEBASTIAN ŁASKAWIEC

INFINISPAN DEVELOPER


_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [infinispan-dev] To Optional or not to Optional?

Sebastian Laskawiec
In reply to this post by Radim Vansa
Adding part of your email from REST refactoring thread:

Looking into the code, without considering performance at all, I think
that you've become too ecstatic about Optionals. These should be used as
return types for methods, not a) parameters to methods nor b) fields.
This is a misuse of the API, according to the authors of Optionals in
JDK. Most of the time, you're not using optionals to have fluent chain
of method invocations, so -100 to that.

I'm sorry I'm not picking up the discussion about REST refactoring PR since it has been already merged. Plus I'm not planning to do any Optionals refactoring as long I don't have a clear vision how we'd like to approach it.

But I'm actually very happy you touched the use case topic. So far we were discussing advantages and disadvantages of Optionals and we didn't say much about potential use cases (Katia, Dan, Galder and Sanne also touched a little this topic). 

Indeed, Stephen Colebourne [1] mentions that it should be used as method return types:
"My only fear is that Optional will be overused. Please focus on using it as a return type (from methods that perform some useful piece of functionality) Please don't use it as the field of a Java-Bean."

Brian Goetz also said a few words on Stack Overflow about this [2]:
"For example, you probably should never use it for something that returns an array of results, or a list of results; instead return an empty array or list. You should almost never use it as a field of something or a method parameter.
I think routinely using it as a return value for getters would definitely be over-use."

So if we want to be really dogmatic here, we wouldn't be able to use Optionals in fields, method parameters, and getters. Please note that I'm blindly putting recommendations mentioned above into code. As it turns out we can use Optionals anywhere, except method returning some objects which are not getters.

It is also worth to say that both gentlemen are worried that Optionals might be overused in the libraries.

On the other hand we have Oracle's tutorials which use Optionals as a fields [3]:
"public class Soundcard {
  private Optional<USB> usb;
  public Optional<USB> getUSB() { ... }
}"
and say no word about recommendations mentioned in [1] and [2].

Also many libraries (like Jackson, Hibernate validator) support Optionals as fields [5]. So it must be somewhat popular use case right?

I think my favorit reading about Optional use cases is this [6]. So the author suggests to use Optionals as a return types in API boundaries but use nulls inside classes. This has two major advantages:
  • It makes the library caller aware that the value might not be there
  • The returned Optional object will probably die very soon (a called will probably do something with it right away)
An example based on Oracle's tutorial would look like this (following this recommendation):
"public class Soundcard {
  private USB usb;
  public Optional<USB> getUSB() { return Optional.ofNullable(usb); }
}"

I think it hits exactly into Katia's, Sanne's, Dan's and Galder's points.

What do you think?


On Wed, May 24, 2017 at 4:56 PM Radim Vansa <[hidden email]> wrote:
I haven't checked Sebastian's refactored code, but does it use Optionals
as a *field* type? That's misuse (same as using it as an arg), it's
intended solely as method return type.

Radim

On 05/23/2017 05:45 PM, Katia Aresti wrote:
> Dan, I disagree with point 2 where you say "You now have a field that
> could be null, Optional.empty(), or Optional.of(something)"
>
> This is the point of optional. You shouldn't have a field that has
> these 3 possible values, just two of them = Some or None. If the field
> is mutable, it should be initialised to Optional.empty(). In the case
> of an API, Optional implicitly says that the return value can be
> empty, but when you return a "normal" object, either the user reads
> the doc, either will have bugs or boilerplate code defending from the
> possible null value (even if never ever this API will return null)
>
> :o)
>
> Cheers
>
>
>
> On Tue, May 23, 2017 at 3:58 PM, Dan Berindei <[hidden email]
> <mailto:[hidden email]>> wrote:
>
>     I wouldn't say I'm an extreme naysayer, but I do have 2 issues
>     with Optional:
>
>     1. Performance becomes harder to quantify: the allocations may or
>     may not be eliminated, and a change in one part of the code may
>     change how allocations are eliminated in a completely different
>     part of the code.
>     2. My personal opinion is it's just ugly... instead of having one
>     field that could be null or non-null, you now have a field that
>     could be null, Optional.empty(), or Optional.of(something).
>
>     Cheers
>     Dan
>
>
>
>     On Tue, May 23, 2017 at 1:54 PM, Sebastian Laskawiec
>     <[hidden email] <mailto:[hidden email]>> wrote:
>
>         Hey!
>
>         So I think we have no extreme naysayers to Optional. So let me
>         try to sum up what we have achieved so:
>
>           * In macroscale benchmark based on REST interface using
>             Optionals didn't lower the performance.
>           * +1 for using it in public APIs, especially for those using
>             functional style.
>           * Creating lots of Optional instances might add some
>             pressure on GC, so we need to be careful when using them
>             in hot code paths. In such cases it is required to run a
>             micro scale benchamark to make sure the performance didn't
>             drop. The microbenchmark should also be followed by macro
>             scale benchamrk - PerfJobAck. Also, keep an eye on Eden
>             space in such cases.
>
>         If you agree with me, and there are no hard evidence that
>         using Optional degrade performance significantly, I would like
>         to issue a pull request and put those findings into
>         contributing guide [1].
>
>         Thanks,
>         Sebastian
>
>         [1]
>         https://github.com/infinispan/infinispan/tree/master/documentation/src/main/asciidoc/contributing
>         <https://github.com/infinispan/infinispan/tree/master/documentation/src/main/asciidoc/contributing>
>
>         On Mon, May 22, 2017 at 6:36 PM Galder Zamarreño
>         <[hidden email] <mailto:[hidden email]>> wrote:
>
>             I think Sanne's right here, any differences in such large
>             scale test are hard to decipher.
>
>             Also, as mentioned in a previous email, my view on its
>             usage is same as Sanne's:
>
>             * Definitely in APIs/SPIs.
>             * Be gentle with it internals.
>
>             Cheers,
>             --
>             Galder Zamarreño
>             Infinispan, Red Hat
>
>             > On 18 May 2017, at 14:35, Sanne Grinovero
>             <[hidden email] <mailto:[hidden email]>> wrote:
>             >
>             > Hi Sebastian,
>             >
>             > sorry but I think you've been wasting time, I hope it
>             was fun :) This is not the right methodology to "settle"
>             the matter (unless you want Radim's eyes to get bloody..).
>             >
>             > Any change in such a complex system will only affect the
>             performance metrics if you're actually addressing the
>             dominant bottleneck. In some cases it might be CPU, like
>             if your system is at 90%+ CPU then it's likely that
>             reviewing the code to use less CPU would be beneficial;
>             but even that can be counter-productive, for example if
>             you're having contention caused by optimistic locking and
>             you fail to address that while making something else
>             "faster" the performance loss on the optimistic lock might
>             become asymptotic.
>             >
>             > A good reason to avoid excessive usage of Optional (and
>             *excessive* doesn't mean a couple dozen in a millions
>             lines of code..) is to not run out of eden space,
>             especially for all the code running in interpreted mode.
>             >
>             > In your case you've been benchmarking a hugely complex
>             beast, not least over REST! When running the REST Server I
>             doubt that allocation in eden is your main problem. You
>             just happened to have a couple Optionals on your path;
>             sure performance changed but there's no enough data in
>             this way to figure out what exactly happened:
>             >  - did it change at all or was it just because of a
>             lucky optimisation? (The JIT will always optimise stuff
>             differently even when re-running the same code)
>             >  - did the overall picture improve because this code
>             became much *less* slower?
>             >
>             > The real complexity in benchmarking is to accurately
>             understand why it changed; this should also tell you why
>             it didn't change more, or less..
>             >
>             > To be fair I actually agree that it's very likely that
>             C2 can make any performance penalty disappear.. that's
>             totally possible, although it's unlikely to be faster than
>             just reading the field (assuming we don't need to do
>             branching because of null-checks but C2 can optimise that
>             as well).
>             > Still this requires the code to be optimised by JIT
>             first, so it won't prevent us from creating a gazillion of
>             instances if we abuse its usage irresponsibly. Fighting
>             internal NPEs is a matter of writing better code; I'm not
>             against some "Optional" being strategically placed but I
>             believe it's much nicer for most internal code to just
>             avoid null, use "final", and initialize things aggressively.
>             >
>             > Sure use Optional where it makes sense, probably most on
>             APIs and SPIs, but please don't go overboard with it in
>             internals. That's all I said in the original debate.
>             >
>             > In case you want to benchmark the impact of Optional
>             make a JMH based microbenchmark - that's interesting to
>             see what C2 is capable of - but even so that's not going
>             to tell you much on the impact it would have to patch
>             thousands of code all around Infinispan. And it will need
>             some peer review before it can tell you anything at all ;)
>             >
>             > It's actually a very challenging topic, as we produce
>             libraries meant for "anyone to use" and don't get to set
>             the hardware specification requirements it's hard to
>             predict if we should optimise the system for this/that
>             resource consumption. Some people will have plenty of CPU
>             and have problems with us needing too much memory, some
>             others will have the opposite.. the real challenge is in
>             making internals "elastic" to such factors and adaptable
>             without making it too hard to tune.
>             >
>             > Thanks,
>             > Sanne
>             >
>             >
>             >
>             > On 18 May 2017 at 12:30, Sebastian Laskawiec
>             <[hidden email] <mailto:[hidden email]>> wrote:
>             > Hey!
>             >
>             > In our past we had a couple of discussions about whether
>             we should or should not use Optionals [1][2]. The main
>             argument against it was performance.
>             >
>             > On one hand we risk additional object allocation (the
>             Optional itself) and wrong inlining decisions taken by C2
>             compiler [3]. On the other hand we all probably "feel"
>             that both of those things shouldn't be a problem and
>             should be optimized by C2. Another argument was the
>             Optional's doesn't give us anything but as I checked, we
>             introduced nearly 80 NullPointerException bugs in two
>             years [4]. So we might consider Optional as a way of
>             fighting those things. The final argument that I've seen
>             was about lack of higher order functions which is simply
>             not true since we have #map, #filter and #flatmap
>             functions. You can do pretty amazing things with this.
>             >
>             > I decided to check the performance when refactoring REST
>             interface. I created a PR with Optionals [5], ran
>             performance tests, removed all Optionals and reran tests.
>             You will be surprised by the results [6]:
>             >
>             > Test case
>             > With Optionals [%]    Without Optionals
>             > Run 1 Run 2   Avg     Run 1   Run 2   Avg
>             > Non-TX reads 10 threads
>             > Throughput    32.54   32.87  32.71   31.74   34.04   32.89
>             > Response time -24.12  -24.63 -24.38  -24.37  -25.69  -25.03
>             > Non-TX reads 100 threads
>             > Throughput    6.48    -12.79 -3.16   -7.06   -6.14   -6.60
>             > Response time -6.15   14.93   4.39   7.88    6.49    7.19
>             > Non-TX writes 10 threads
>             > Throughput    9.21    7.60    8.41   4.66    7.15    5.91
>             > Response time -8.92   -7.11  -8.02   -5.29   -6.93   -6.11
>             > Non-TX writes 100 threads
>             > Throughput    2.53    1.65    2.09   -1.16   4.67    1.76
>             > Response time -2.13   -1.79  -1.96   0.91    -4.67   -1.88
>             >
>             > I also created JMH + Flight Recorder tests and again,
>             the results showed no evidence of slow down caused by
>             Optionals [7].
>             >
>             > Now please take those results with a grain of salt since
>             they tend to drift by a factor of +/-5% (sometimes even
>             more). But it's very clear the performance results are
>             very similar if not the same.
>             >
>             > Having those numbers at hand, do we want to have
>             Optionals in Infinispan codebase or not? And if not, let's
>             state it very clearly (and write it into contributing
>             guide), it's because we don't like them. Not because of
>             performance.
>             >
>             > Thanks,
>             > Sebastian
>             >
>             > [1]
>             http://lists.jboss.org/pipermail/infinispan-dev/2017-March/017370.html
>             <http://lists.jboss.org/pipermail/infinispan-dev/2017-March/017370.html>
>             > [2]
>             http://lists.jboss.org/pipermail/infinispan-dev/2016-August/016796.html
>             <http://lists.jboss.org/pipermail/infinispan-dev/2016-August/016796.html>
>             > [3]
>             http://vanillajava.blogspot.ro/2015/01/java-lambdas-and-low-latency.html
>             <http://vanillajava.blogspot.ro/2015/01/java-lambdas-and-low-latency.html>
>             > [4]
>             https://issues.jboss.org/issues/?jql=project%20%3D%20ISPN%20AND%20issuetype%20%3D%20Bug%20AND%20text%20%7E%20%22NullPointerException%22%20AND%20created%20%3E%3D%202015-04-27%20AND%20created%20%3C%3D%202017-04-27
>             <https://issues.jboss.org/issues/?jql=project%20%3D%20ISPN%20AND%20issuetype%20%3D%20Bug%20AND%20text%20%7E%20%22NullPointerException%22%20AND%20created%20%3E%3D%202015-04-27%20AND%20created%20%3C%3D%202017-04-27>
>             > [5] https://github.com/infinispan/infinispan/pull/5094
>             <https://github.com/infinispan/infinispan/pull/5094>
>             > [6]
>             https://docs.google.com/a/redhat.com/spreadsheets/d/1oep6Was0FfvHdqBCwpCFIqcPfJZ5-5_YYUqlRtUxEkM/edit?usp=sharing
>             <https://docs.google.com/a/redhat.com/spreadsheets/d/1oep6Was0FfvHdqBCwpCFIqcPfJZ5-5_YYUqlRtUxEkM/edit?usp=sharing>
>             > [7]
>             https://github.com/infinispan/infinispan/pull/5094#issuecomment-296970673
>             <https://github.com/infinispan/infinispan/pull/5094#issuecomment-296970673>
>             > --
>             > SEBASTIAN ŁASKAWIEC
>             > INFINISPAN DEVELOPER
>             > Red Hat EMEA
>             >
>             >
>             > _______________________________________________
>             > infinispan-dev mailing list
>             > [hidden email]
>             <mailto:[hidden email]>
>             > https://lists.jboss.org/mailman/listinfo/infinispan-dev
>             <https://lists.jboss.org/mailman/listinfo/infinispan-dev>
>             >
>             > _______________________________________________
>             > infinispan-dev mailing list
>             > [hidden email]
>             <mailto:[hidden email]>
>             > https://lists.jboss.org/mailman/listinfo/infinispan-dev
>             <https://lists.jboss.org/mailman/listinfo/infinispan-dev>
>
>
>             _______________________________________________
>             infinispan-dev mailing list
>             [hidden email]
>             <mailto:[hidden email]>
>             https://lists.jboss.org/mailman/listinfo/infinispan-dev
>             <https://lists.jboss.org/mailman/listinfo/infinispan-dev>
>
>         --
>
>         SEBASTIANŁASKAWIEC
>
>         INFINISPAN DEVELOPER
>
>         Red HatEMEA <https://www.redhat.com/>
>
>         <https://red.ht/sig>
>
>
>         _______________________________________________
>         infinispan-dev mailing list
>         [hidden email]
>         <mailto:[hidden email]>
>         https://lists.jboss.org/mailman/listinfo/infinispan-dev
>         <https://lists.jboss.org/mailman/listinfo/infinispan-dev>
>
>
>
>     _______________________________________________
>     infinispan-dev mailing list
>     [hidden email] <mailto:[hidden email]>
>     https://lists.jboss.org/mailman/listinfo/infinispan-dev
>     <https://lists.jboss.org/mailman/listinfo/infinispan-dev>
>
>
>
>
> _______________________________________________
> infinispan-dev mailing list
> [hidden email]
> https://lists.jboss.org/mailman/listinfo/infinispan-dev


--
Radim Vansa <[hidden email]>
JBoss Performance Team

_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev
--

SEBASTIAN ŁASKAWIEC

INFINISPAN DEVELOPER


_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [infinispan-dev] To Optional or not to Optional?

Emmanuel Bernard


On 25 May 2017, at 10:00, Sebastian Laskawiec <[hidden email]> wrote:
. As it turns out we can use Optionals anywhere, except method returning some objects which are not getters.
You can't use it on non getter return types ? Why ?


It is also worth to say that both gentlemen are worried that Optionals might be overused in the libraries.

On the other hand we have Oracle's tutorials which use Optionals as a fields [3]:
"public class Soundcard {
  private Optional<USB> usb;
  public Optional<USB> getUSB() { ... }
}"
and say no word about recommendations mentioned in [1] and [2].

Yes but tutorials are not written by the best possible people. I would not use them as gospel. 


Also many libraries (like Jackson, Hibernate validator) support Optionals as fields [5]. So it must be somewhat popular use case right?

For Hibernate validator we added the support because we do support validation of return types. And instead of making an opinionated spec we let the support for params and fields slip in as it was more regular for us anyways. 
So don't take it as a measure of popularity or endorsement. 


I think my favorit reading about Optional use cases is this [6]. So the author suggests to use Optionals as a return types in API boundaries but use nulls inside classes. This has two major advantages:
  • It makes the library caller aware that the value might not be there
  • The returned Optional object will probably die very soon (a called will probably do something with it right away)
An example based on Oracle's tutorial would look like this (following this recommendation):
"public class Soundcard {
  private USB usb;
  public Optional<USB> getUSB() { return Optional.ofNullable(usb); }
}"

I think it hits exactly into Katia's, Sanne's, Dan's and Galder's points.

What do you think?


On Wed, May 24, 2017 at 4:56 PM Radim Vansa <[hidden email]> wrote:
I haven't checked Sebastian's refactored code, but does it use Optionals
as a *field* type? That's misuse (same as using it as an arg), it's
intended solely as method return type.

Radim

On 05/23/2017 05:45 PM, Katia Aresti wrote:
> Dan, I disagree with point 2 where you say "You now have a field that
> could be null, Optional.empty(), or Optional.of(something)"
>
> This is the point of optional. You shouldn't have a field that has
> these 3 possible values, just two of them = Some or None. If the field
> is mutable, it should be initialised to Optional.empty(). In the case
> of an API, Optional implicitly says that the return value can be
> empty, but when you return a "normal" object, either the user reads
> the doc, either will have bugs or boilerplate code defending from the
> possible null value (even if never ever this API will return null)
>
> :o)
>
> Cheers
>
>
>
> On Tue, May 23, 2017 at 3:58 PM, Dan Berindei <[hidden email]
> <mailto:[hidden email]>> wrote:
>
>     I wouldn't say I'm an extreme naysayer, but I do have 2 issues
>     with Optional:
>
>     1. Performance becomes harder to quantify: the allocations may or
>     may not be eliminated, and a change in one part of the code may
>     change how allocations are eliminated in a completely different
>     part of the code.
>     2. My personal opinion is it's just ugly... instead of having one
>     field that could be null or non-null, you now have a field that
>     could be null, Optional.empty(), or Optional.of(something).
>
>     Cheers
>     Dan
>
>
>
>     On Tue, May 23, 2017 at 1:54 PM, Sebastian Laskawiec
>     <[hidden email] <mailto:[hidden email]>> wrote:
>
>         Hey!
>
>         So I think we have no extreme naysayers to Optional. So let me
>         try to sum up what we have achieved so:
>
>           * In macroscale benchmark based on REST interface using
>             Optionals didn't lower the performance.
>           * +1 for using it in public APIs, especially for those using
>             functional style.
>           * Creating lots of Optional instances might add some
>             pressure on GC, so we need to be careful when using them
>             in hot code paths. In such cases it is required to run a
>             micro scale benchamark to make sure the performance didn't
>             drop. The microbenchmark should also be followed by macro
>             scale benchamrk - PerfJobAck. Also, keep an eye on Eden
>             space in such cases.
>
>         If you agree with me, and there are no hard evidence that
>         using Optional degrade performance significantly, I would like
>         to issue a pull request and put those findings into
>         contributing guide [1].
>
>         Thanks,
>         Sebastian
>
>         [1]
>         https://github.com/infinispan/infinispan/tree/master/documentation/src/main/asciidoc/contributing
>         <https://github.com/infinispan/infinispan/tree/master/documentation/src/main/asciidoc/contributing>
>
>         On Mon, May 22, 2017 at 6:36 PM Galder Zamarreño
>         <[hidden email] <mailto:[hidden email]>> wrote:
>
>             I think Sanne's right here, any differences in such large
>             scale test are hard to decipher.
>
>             Also, as mentioned in a previous email, my view on its
>             usage is same as Sanne's:
>
>             * Definitely in APIs/SPIs.
>             * Be gentle with it internals.
>
>             Cheers,
>             --
>             Galder Zamarreño
>             Infinispan, Red Hat
>
>             > On 18 May 2017, at 14:35, Sanne Grinovero
>             <[hidden email] <mailto:[hidden email]>> wrote:
>             >
>             > Hi Sebastian,
>             >
>             > sorry but I think you've been wasting time, I hope it
>             was fun :) This is not the right methodology to "settle"
>             the matter (unless you want Radim's eyes to get bloody..).
>             >
>             > Any change in such a complex system will only affect the
>             performance metrics if you're actually addressing the
>             dominant bottleneck. In some cases it might be CPU, like
>             if your system is at 90%+ CPU then it's likely that
>             reviewing the code to use less CPU would be beneficial;
>             but even that can be counter-productive, for example if
>             you're having contention caused by optimistic locking and
>             you fail to address that while making something else
>             "faster" the performance loss on the optimistic lock might
>             become asymptotic.
>             >
>             > A good reason to avoid excessive usage of Optional (and
>             *excessive* doesn't mean a couple dozen in a millions
>             lines of code..) is to not run out of eden space,
>             especially for all the code running in interpreted mode.
>             >
>             > In your case you've been benchmarking a hugely complex
>             beast, not least over REST! When running the REST Server I
>             doubt that allocation in eden is your main problem. You
>             just happened to have a couple Optionals on your path;
>             sure performance changed but there's no enough data in
>             this way to figure out what exactly happened:
>             >  - did it change at all or was it just because of a
>             lucky optimisation? (The JIT will always optimise stuff
>             differently even when re-running the same code)
>             >  - did the overall picture improve because this code
>             became much *less* slower?
>             >
>             > The real complexity in benchmarking is to accurately
>             understand why it changed; this should also tell you why
>             it didn't change more, or less..
>             >
>             > To be fair I actually agree that it's very likely that
>             C2 can make any performance penalty disappear.. that's
>             totally possible, although it's unlikely to be faster than
>             just reading the field (assuming we don't need to do
>             branching because of null-checks but C2 can optimise that
>             as well).
>             > Still this requires the code to be optimised by JIT
>             first, so it won't prevent us from creating a gazillion of
>             instances if we abuse its usage irresponsibly. Fighting
>             internal NPEs is a matter of writing better code; I'm not
>             against some "Optional" being strategically placed but I
>             believe it's much nicer for most internal code to just
>             avoid null, use "final", and initialize things aggressively.
>             >
>             > Sure use Optional where it makes sense, probably most on
>             APIs and SPIs, but please don't go overboard with it in
>             internals. That's all I said in the original debate.
>             >
>             > In case you want to benchmark the impact of Optional
>             make a JMH based microbenchmark - that's interesting to
>             see what C2 is capable of - but even so that's not going
>             to tell you much on the impact it would have to patch
>             thousands of code all around Infinispan. And it will need
>             some peer review before it can tell you anything at all ;)
>             >
>             > It's actually a very challenging topic, as we produce
>             libraries meant for "anyone to use" and don't get to set
>             the hardware specification requirements it's hard to
>             predict if we should optimise the system for this/that
>             resource consumption. Some people will have plenty of CPU
>             and have problems with us needing too much memory, some
>             others will have the opposite.. the real challenge is in
>             making internals "elastic" to such factors and adaptable
>             without making it too hard to tune.
>             >
>             > Thanks,
>             > Sanne
>             >
>             >
>             >
>             > On 18 May 2017 at 12:30, Sebastian Laskawiec
>             <[hidden email] <mailto:[hidden email]>> wrote:
>             > Hey!
>             >
>             > In our past we had a couple of discussions about whether
>             we should or should not use Optionals [1][2]. The main
>             argument against it was performance.
>             >
>             > On one hand we risk additional object allocation (the
>             Optional itself) and wrong inlining decisions taken by C2
>             compiler [3]. On the other hand we all probably "feel"
>             that both of those things shouldn't be a problem and
>             should be optimized by C2. Another argument was the
>             Optional's doesn't give us anything but as I checked, we
>             introduced nearly 80 NullPointerException bugs in two
>             years [4]. So we might consider Optional as a way of
>             fighting those things. The final argument that I've seen
>             was about lack of higher order functions which is simply
>             not true since we have #map, #filter and #flatmap
>             functions. You can do pretty amazing things with this.
>             >
>             > I decided to check the performance when refactoring REST
>             interface. I created a PR with Optionals [5], ran
>             performance tests, removed all Optionals and reran tests.
>             You will be surprised by the results [6]:
>             >
>             > Test case
>             > With Optionals [%]    Without Optionals
>             > Run 1 Run 2   Avg     Run 1   Run 2   Avg
>             > Non-TX reads 10 threads
>             > Throughput    32.54   32.87  32.71   31.74   34.04   32.89
>             > Response time -24.12  -24.63 -24.38  -24.37  -25.69  -25.03
>             > Non-TX reads 100 threads
>             > Throughput    6.48    -12.79 -3.16   -7.06   -6.14   -6.60
>             > Response time -6.15   14.93   4.39   7.88    6.49    7.19
>             > Non-TX writes 10 threads
>             > Throughput    9.21    7.60    8.41   4.66    7.15    5.91
>             > Response time -8.92   -7.11  -8.02   -5.29   -6.93   -6.11
>             > Non-TX writes 100 threads
>             > Throughput    2.53    1.65    2.09   -1.16   4.67    1.76
>             > Response time -2.13   -1.79  -1.96   0.91    -4.67   -1.88
>             >
>             > I also created JMH + Flight Recorder tests and again,
>             the results showed no evidence of slow down caused by
>             Optionals [7].
>             >
>             > Now please take those results with a grain of salt since
>             they tend to drift by a factor of +/-5% (sometimes even
>             more). But it's very clear the performance results are
>             very similar if not the same.
>             >
>             > Having those numbers at hand, do we want to have
>             Optionals in Infinispan codebase or not? And if not, let's
>             state it very clearly (and write it into contributing
>             guide), it's because we don't like them. Not because of
>             performance.
>             >
>             > Thanks,
>             > Sebastian
>             >
>             > [1]
>             http://lists.jboss.org/pipermail/infinispan-dev/2017-March/017370.html
>             <http://lists.jboss.org/pipermail/infinispan-dev/2017-March/017370.html>
>             > [2]
>             http://lists.jboss.org/pipermail/infinispan-dev/2016-August/016796.html
>             <http://lists.jboss.org/pipermail/infinispan-dev/2016-August/016796.html>
>             > [3]
>             http://vanillajava.blogspot.ro/2015/01/java-lambdas-and-low-latency.html
>             <http://vanillajava.blogspot.ro/2015/01/java-lambdas-and-low-latency.html>
>             > [4]
>             https://issues.jboss.org/issues/?jql=project%20%3D%20ISPN%20AND%20issuetype%20%3D%20Bug%20AND%20text%20%7E%20%22NullPointerException%22%20AND%20created%20%3E%3D%202015-04-27%20AND%20created%20%3C%3D%202017-04-27
>             <https://issues.jboss.org/issues/?jql=project%20%3D%20ISPN%20AND%20issuetype%20%3D%20Bug%20AND%20text%20%7E%20%22NullPointerException%22%20AND%20created%20%3E%3D%202015-04-27%20AND%20created%20%3C%3D%202017-04-27>
>             > [5] https://github.com/infinispan/infinispan/pull/5094
>             <https://github.com/infinispan/infinispan/pull/5094>
>             > [6]
>             https://docs.google.com/a/redhat.com/spreadsheets/d/1oep6Was0FfvHdqBCwpCFIqcPfJZ5-5_YYUqlRtUxEkM/edit?usp=sharing
>             <https://docs.google.com/a/redhat.com/spreadsheets/d/1oep6Was0FfvHdqBCwpCFIqcPfJZ5-5_YYUqlRtUxEkM/edit?usp=sharing>
>             > [7]
>             https://github.com/infinispan/infinispan/pull/5094#issuecomment-296970673
>             <https://github.com/infinispan/infinispan/pull/5094#issuecomment-296970673>
>             > --
>             > SEBASTIAN ŁASKAWIEC
>             > INFINISPAN DEVELOPER
>             > Red Hat EMEA
>             >
>             >
>             > _______________________________________________
>             > infinispan-dev mailing list
>             > [hidden email]
>             <mailto:[hidden email]>
>             > https://lists.jboss.org/mailman/listinfo/infinispan-dev
>             <https://lists.jboss.org/mailman/listinfo/infinispan-dev>
>             >
>             > _______________________________________________
>             > infinispan-dev mailing list
>             > [hidden email]
>             <mailto:[hidden email]>
>             > https://lists.jboss.org/mailman/listinfo/infinispan-dev
>             <https://lists.jboss.org/mailman/listinfo/infinispan-dev>
>
>
>             _______________________________________________
>             infinispan-dev mailing list
>             [hidden email]
>             <mailto:[hidden email]>
>             https://lists.jboss.org/mailman/listinfo/infinispan-dev
>             <https://lists.jboss.org/mailman/listinfo/infinispan-dev>
>
>         --
>
>         SEBASTIANŁASKAWIEC
>
>         INFINISPAN DEVELOPER
>
>         Red HatEMEA <https://www.redhat.com/>
>
>         <https://red.ht/sig>
>
>
>         _______________________________________________
>         infinispan-dev mailing list
>         [hidden email]
>         <mailto:[hidden email]>
>         https://lists.jboss.org/mailman/listinfo/infinispan-dev
>         <https://lists.jboss.org/mailman/listinfo/infinispan-dev>
>
>
>
>     _______________________________________________
>     infinispan-dev mailing list
>     [hidden email] <mailto:[hidden email]>
>     https://lists.jboss.org/mailman/listinfo/infinispan-dev
>     <https://lists.jboss.org/mailman/listinfo/infinispan-dev>
>
>
>
>
> _______________________________________________
> infinispan-dev mailing list
> [hidden email]
> https://lists.jboss.org/mailman/listinfo/infinispan-dev


--
Radim Vansa <[hidden email]>
JBoss Performance Team

_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev
--

SEBASTIAN ŁASKAWIEC

INFINISPAN DEVELOPER

_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev

_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [infinispan-dev] To Optional or not to Optional?

David M. Lloyd
In reply to this post by Sebastian Laskawiec
I'm not an Infinispan developer, but I'll chime in anyway. :)

I've never been a fan of Optional.  But the theory behind it that made
it acceptable in the first place is that it generally gets optimized away.

Now this theory is only true if you never hold a reference to it in any
persistent manner (it should be very short-lived even as a local
variable (after optimizations like dead-code & etc. have run)).

What should happen is, the actual allocation should get deleted, and the
(usually one or two) strictly monomorphic or bi-morphic call(s) should
each be flattened into what amounts to (at most) simple if/else
statement(s), all of which should be very fast (as fast as a null-check,
in theory) and branch-predictable and all that good stuff.  (Now null
checks have a very slight advantage in that they can be optimistically
removed in some cases, and only re-added once the operating system gets
a SIGSEGV or equivalent, but that difference is usually going to be
pretty small even in fairly tightly optimized code).  This seems
consistent with the benchmark results.  I doubt it has much to do with
how hot the code path is (in fact, a hotter code path should mean
Optional usages will get more accurately optimized by C2 over time).
With an appropriate JDK build, you can browse the compiler output to
test this theory out.

If you put an Optional into a field, all of this is very likely to be
thrown in the garbage.  I think there is some escape analysis stuff that
might apply but in the most likely case, the heap will simply be
polluted with a bunch useless crap, along with all the consequences thereof.

So if (and only if) you're using Optional as a return value, creating it
on the fly (particularly in monomorphic methods), and using the result
directly via the chainable methods on the Optional class or keeping it
around only for one or two usages in a local variable (not referring to
it afterwards in any way), it *should* be fine from a performance
perspective.  Note that HotSpot is pretty good at knowing the difference
between when *you* think the value is being referred to and when the
value is *really* being referred to (this is what caused the finalize()
debacle that resulted in Runtime.reachabilityFence() being added to Java
9 - HotSpot was a little *too* good at it).

Aesthetically it's a different story.  There's no magic silver bullet to
make null problems go away; Optional is a tradeoff just like everything
in engineering and in life.  I would never put Optional into one of my
APIs as it exists today (or even as it exists in Java 9, where several
deficiencies have admittedly been addressed).  I would only start using
it once it is extremely well-understood and well-established (i.e. maybe
in 5-10 years I'll have another look).

On 05/25/2017 02:56 AM, Sebastian Laskawiec wrote:

> Indeed Bela, you're an extreme naysayer! :)
>
> I'm actually trying to get as many comments and arguments out of this
> discussion. I hope we will be able to iron out a general recommendation
> or approach how we want to treat Optionals.
>
> On Tue, May 23, 2017 at 10:14 PM Bela Ban <[hidden email]
> <mailto:[hidden email]>> wrote:
>
>     Actually, I'm an extreme naysayer! I actually voiced concerns so I'm
>     wondering where your assumption there are no naysayers is coming
>     from... :-)
>
>
>     On 23/05/17 1:54 PM, Sebastian Laskawiec wrote:
>      > Hey!
>      >
>      > So I think we have no extreme naysayers to Optional. So let me try to
>      > sum up what we have achieved so:
>      >
>      >   * In macroscale benchmark based on REST interface using Optionals
>      >     didn't lower the performance.
>      >   * +1 for using it in public APIs, especially for those using
>      >     functional style.
>      >   * Creating lots of Optional instances might add some pressure
>     on GC,
>      >     so we need to be careful when using them in hot code paths. In
>      >     such cases it is required to run a micro scale benchamark to make
>      >     sure the performance didn't drop. The microbenchmark should also
>      >     be followed by macro scale benchamrk - PerfJobAck. Also, keep an
>      >     eye on Eden space in such cases.
>      >
>      > If you agree with me, and there are no hard evidence that using
>      > Optional degrade performance significantly, I would like to issue a
>      > pull request and put those findings into contributing guide [1].
>      >
>      > Thanks,
>      > Sebastian
>      >
>      > [1]
>      >
>     https://github.com/infinispan/infinispan/tree/master/documentation/src/main/asciidoc/contributing
>      >
>      > On Mon, May 22, 2017 at 6:36 PM Galder Zamarreño
>     <[hidden email] <mailto:[hidden email]>
>      > <mailto:[hidden email] <mailto:[hidden email]>>> wrote:
>      >
>      >     I think Sanne's right here, any differences in such large scale
>      >     test are hard to decipher.
>      >
>      >     Also, as mentioned in a previous email, my view on its usage is
>      >     same as Sanne's:
>      >
>      >     * Definitely in APIs/SPIs.
>      >     * Be gentle with it internals.
>      >
>      >     Cheers,
>      >     --
>      >     Galder Zamarreño
>      >     Infinispan, Red Hat
>      >
>      >     > On 18 May 2017, at 14:35, Sanne Grinovero
>     <[hidden email] <mailto:[hidden email]>
>      >     <mailto:[hidden email] <mailto:[hidden email]>>>
>     wrote:
>      >     >
>      >     > Hi Sebastian,
>      >     >
>      >     > sorry but I think you've been wasting time, I hope it was
>     fun :)
>      >     This is not the right methodology to "settle" the matter (unless
>      >     you want Radim's eyes to get bloody..).
>      >     >
>      >     > Any change in such a complex system will only affect the
>      >     performance metrics if you're actually addressing the dominant
>      >     bottleneck. In some cases it might be CPU, like if your system is
>      >     at 90%+ CPU then it's likely that reviewing the code to use less
>      >     CPU would be beneficial; but even that can be counter-productive,
>      >     for example if you're having contention caused by optimistic
>      >     locking and you fail to address that while making something else
>      >     "faster" the performance loss on the optimistic lock might become
>      >     asymptotic.
>      >     >
>      >     > A good reason to avoid excessive usage of Optional (and
>      >     *excessive* doesn't mean a couple dozen in a millions lines of
>      >     code..) is to not run out of eden space, especially for all the
>      >     code running in interpreted mode.
>      >     >
>      >     > In your case you've been benchmarking a hugely complex beast,
>      >     not least over REST! When running the REST Server I doubt that
>      >     allocation in eden is your main problem. You just happened to
>     have
>      >     a couple Optionals on your path; sure performance changed but
>      >     there's no enough data in this way to figure out what exactly
>      >     happened:
>      >     >  - did it change at all or was it just because of a lucky
>      >     optimisation? (The JIT will always optimise stuff differently
>     even
>      >     when re-running the same code)
>      >     >  - did the overall picture improve because this code became
>     much
>      >     *less* slower?
>      >     >
>      >     > The real complexity in benchmarking is to accurately understand
>      >     why it changed; this should also tell you why it didn't change
>      >     more, or less..
>      >     >
>      >     > To be fair I actually agree that it's very likely that C2 can
>      >     make any performance penalty disappear.. that's totally possible,
>      >     although it's unlikely to be faster than just reading the field
>      >     (assuming we don't need to do branching because of
>     null-checks but
>      >     C2 can optimise that as well).
>      >     > Still this requires the code to be optimised by JIT first,
>     so it
>      >     won't prevent us from creating a gazillion of instances if we
>      >     abuse its usage irresponsibly. Fighting internal NPEs is a matter
>      >     of writing better code; I'm not against some "Optional" being
>      >     strategically placed but I believe it's much nicer for most
>      >     internal code to just avoid null, use "final", and initialize
>      >     things aggressively.
>      >     >
>      >     > Sure use Optional where it makes sense, probably most on APIs
>      >     and SPIs, but please don't go overboard with it in internals.
>      >     That's all I said in the original debate.
>      >     >
>      >     > In case you want to benchmark the impact of Optional make a JMH
>      >     based microbenchmark - that's interesting to see what C2 is
>      >     capable of - but even so that's not going to tell you much on the
>      >     impact it would have to patch thousands of code all around
>      >     Infinispan. And it will need some peer review before it can tell
>      >     you anything at all ;)
>      >     >
>      >     > It's actually a very challenging topic, as we produce libraries
>      >     meant for "anyone to use" and don't get to set the hardware
>      >     specification requirements it's hard to predict if we should
>      >     optimise the system for this/that resource consumption. Some
>      >     people will have plenty of CPU and have problems with us needing
>      >     too much memory, some others will have the opposite.. the real
>      >     challenge is in making internals "elastic" to such factors and
>      >     adaptable without making it too hard to tune.
>      >     >
>      >     > Thanks,
>      >     > Sanne
>      >     >
>      >     >
>      >     >
>      >     > On 18 May 2017 at 12:30, Sebastian Laskawiec
>      >     <[hidden email] <mailto:[hidden email]>
>     <mailto:[hidden email] <mailto:[hidden email]>>> wrote:
>      >     > Hey!
>      >     >
>      >     > In our past we had a couple of discussions about whether we
>      >     should or should not use Optionals [1][2]. The main argument
>      >     against it was performance.
>      >     >
>      >     > On one hand we risk additional object allocation (the Optional
>      >     itself) and wrong inlining decisions taken by C2 compiler [3]. On
>      >     the other hand we all probably "feel" that both of those things
>      >     shouldn't be a problem and should be optimized by C2. Another
>      >     argument was the Optional's doesn't give us anything but as I
>      >     checked, we introduced nearly 80 NullPointerException bugs in two
>      >     years [4]. So we might consider Optional as a way of fighting
>      >     those things. The final argument that I've seen was about lack of
>      >     higher order functions which is simply not true since we have
>      >     #map, #filter and #flatmap functions. You can do pretty amazing
>      >     things with this.
>      >     >
>      >     > I decided to check the performance when refactoring REST
>      >     interface. I created a PR with Optionals [5], ran performance
>      >     tests, removed all Optionals and reran tests. You will be
>      >     surprised by the results [6]:
>      >     >
>      >     > Test case
>      >     > With Optionals [%]    Without Optionals
>      >     > Run 1 Run 2   Avg     Run 1   Run 2   Avg
>      >     > Non-TX reads 10 threads
>      >     > Throughput    32.54   32.87   32.71   31.74   34.04  32.89
>      >     > Response time -24.12  -24.63  -24.38  -24.37  -25.69 -25.03
>      >     > Non-TX reads 100 threads
>      >     > Throughput    6.48    -12.79  -3.16   -7.06   -6.14  -6.60
>      >     > Response time -6.15   14.93   4.39    7.88    6.49 7.19
>      >     > Non-TX writes 10 threads
>      >     > Throughput    9.21    7.60    8.41    4.66    7.15 5.91
>      >     > Response time -8.92   -7.11   -8.02   -5.29   -6.93  -6.11
>      >     > Non-TX writes 100 threads
>      >     > Throughput    2.53    1.65    2.09    -1.16   4.67 1.76
>      >     > Response time -2.13   -1.79   -1.96   0.91    -4.67  -1.88
>      >     >
>      >     > I also created JMH + Flight Recorder tests and again, the
>      >     results showed no evidence of slow down caused by Optionals [7].
>      >     >
>      >     > Now please take those results with a grain of salt since they
>      >     tend to drift by a factor of +/-5% (sometimes even more). But
>     it's
>      >     very clear the performance results are very similar if not
>     the same.
>      >     >
>      >     > Having those numbers at hand, do we want to have Optionals in
>      >     Infinispan codebase or not? And if not, let's state it very
>      >     clearly (and write it into contributing guide), it's because we
>      >     don't like them. Not because of performance.
>      >     >
>      >     > Thanks,
>      >     > Sebastian
>      >     >
>      >     > [1]
>      >
>     http://lists.jboss.org/pipermail/infinispan-dev/2017-March/017370.html
>      >     > [2]
>      >
>     http://lists.jboss.org/pipermail/infinispan-dev/2016-August/016796.html
>      >     > [3]
>      >
>     http://vanillajava.blogspot.ro/2015/01/java-lambdas-and-low-latency.html
>      >     > [4]
>      >
>     https://issues.jboss.org/issues/?jql=project%20%3D%20ISPN%20AND%20issuetype%20%3D%20Bug%20AND%20text%20%7E%20%22NullPointerException%22%20AND%20created%20%3E%3D%202015-04-27%20AND%20created%20%3C%3D%202017-04-27
>      >     > [5] https://github.com/infinispan/infinispan/pull/5094
>      >     > [6]
>      >
>     https://docs.google.com/a/redhat.com/spreadsheets/d/1oep6Was0FfvHdqBCwpCFIqcPfJZ5-5_YYUqlRtUxEkM/edit?usp=sharing
>      >     > [7]
>      >
>     https://github.com/infinispan/infinispan/pull/5094#issuecomment-296970673
>      >     > --
>      >     > SEBASTIAN ŁASKAWIEC
>      >     > INFINISPAN DEVELOPER
>      >     > Red Hat EMEA
>      >     >
>      >     >
>      >     > _______________________________________________
>      >     > infinispan-dev mailing list
>      >     > [hidden email]
>     <mailto:[hidden email]>
>      >     <mailto:[hidden email]
>     <mailto:[hidden email]>>
>      >     > https://lists.jboss.org/mailman/listinfo/infinispan-dev
>      >     >
>      >     > _______________________________________________
>      >     > infinispan-dev mailing list
>      >     > [hidden email]
>     <mailto:[hidden email]>
>      >     <mailto:[hidden email]
>     <mailto:[hidden email]>>
>      >     > https://lists.jboss.org/mailman/listinfo/infinispan-dev
>      >
>      >
>      >     _______________________________________________
>      >     infinispan-dev mailing list
>      > [hidden email]
>     <mailto:[hidden email]>
>     <mailto:[hidden email]
>     <mailto:[hidden email]>>
>      > https://lists.jboss.org/mailman/listinfo/infinispan-dev
>      >
>      > --
>      >
>      > SEBASTIANŁASKAWIEC
>      >
>      > INFINISPAN DEVELOPER
>      >
>      > Red HatEMEA <https://www.redhat.com/>
>      >
>      > <https://red.ht/sig>
>      >
>      >
>      >
>      > _______________________________________________
>      > infinispan-dev mailing list
>      > [hidden email]
>     <mailto:[hidden email]>
>      > https://lists.jboss.org/mailman/listinfo/infinispan-dev
>
>     --
>     Bela Ban, JGroups lead (http://www.jgroups.org)
>
>     _______________________________________________
>     infinispan-dev mailing list
>     [hidden email] <mailto:[hidden email]>
>     https://lists.jboss.org/mailman/listinfo/infinispan-dev
>
> --
>
> SEBASTIANŁASKAWIEC
>
> INFINISPAN DEVELOPER
>
> Red HatEMEA <https://www.redhat.com/>
>
> <https://red.ht/sig>
>
>
>
> _______________________________________________
> infinispan-dev mailing list
> [hidden email]
> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>


--
- DML
_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [infinispan-dev] To Optional or not to Optional?

Sanne Grinovero-3
In reply to this post by Sebastian Laskawiec


On 25 May 2017 at 09:00, Sebastian Laskawiec <[hidden email]> wrote:
Adding part of your email from REST refactoring thread:

Looking into the code, without considering performance at all, I think
that you've become too ecstatic about Optionals. These should be used as
return types for methods, not a) parameters to methods nor b) fields.
This is a misuse of the API, according to the authors of Optionals in
JDK. Most of the time, you're not using optionals to have fluent chain
of method invocations, so -100 to that.

I'm sorry I'm not picking up the discussion about REST refactoring PR since it has been already merged. Plus I'm not planning to do any Optionals refactoring as long I don't have a clear vision how we'd like to approach it.

But I'm actually very happy you touched the use case topic. So far we were discussing advantages and disadvantages of Optionals and we didn't say much about potential use cases (Katia, Dan, Galder and Sanne also touched a little this topic). 

Indeed, Stephen Colebourne [1] mentions that it should be used as method return types:
"My only fear is that Optional will be overused. Please focus on using it as a return type (from methods that perform some useful piece of functionality) Please don't use it as the field of a Java-Bean."

Brian Goetz also said a few words on Stack Overflow about this [2]:
"For example, you probably should never use it for something that returns an array of results, or a list of results; instead return an empty array or list. You should almost never use it as a field of something or a method parameter.
I think routinely using it as a return value for getters would definitely be over-use."

So if we want to be really dogmatic here, we wouldn't be able to use Optionals in fields, method parameters, and getters. Please note that I'm blindly putting recommendations mentioned above into code. As it turns out we can use Optionals anywhere, except method returning some objects which are not getters.

It is also worth to say that both gentlemen are worried that Optionals might be overused in the libraries.

On the other hand we have Oracle's tutorials which use Optionals as a fields [3]:
"public class Soundcard {
  private Optional<USB> usb;
  public Optional<USB> getUSB() { ... }
}"
and say no word about recommendations mentioned in [1] and [2].

Also many libraries (like Jackson, Hibernate validator) support Optionals as fields [5]. So it must be somewhat popular use case right?

​No :)
Speaking for Hibernate I can assure you that allowing *a small minority* of people to use them is not the same as encouraging or endorsing it, and certainly doesn't mean that most Hibernate users have Optional fields.
Fact is we DO NOT use Optional much internally, and even supporting this option was a questionable choice that not all of us agreed on.

 

I think my favorit reading about Optional use cases is this [6]. So the author suggests to use Optionals as a return types in API boundaries but use nulls inside classes. This has two major advantages:
  • It makes the library caller aware that the value might not be there
  • The returned Optional object will probably die very soon (a called will probably do something with it right away)
An example based on Oracle's tutorial would look like this (following this recommendation):
"public class Soundcard {
  private USB usb;
  public Optional<USB> getUSB() { return Optional.ofNullable(usb); }
}"

I think it hits exactly into Katia's, Sanne's, Dan's and Galder's points.

What do you think?

Since you​ expressed the desire for a clear directive, let me try formulate one:

We can allow Optional to be used as long as all these conditions are verified:
 - it's a public API
 - it's not possibly affecting performance of an hot spot (e.g. I'd not want it on Cache#get )
 - it looks good for the use case, e.g. allows fluent APIs and functions to work better
 - on return types, and not for types having a cardinality > 1 (Collections and Iterables).

[Needless to say you shall not break backwards compatibility within a major version, so nothing gets changed now]

Additionally I see no problems on having it on internal code which is rarely executed, e.g. configuration parsing and bootstrap, but please don't start on a quest to update the code for the sake of it: if it evolves over time that's fine but reviewing ad-hoc PRs would be a waste of the team's time.

Thanks,
Sanne

 


On Wed, May 24, 2017 at 4:56 PM Radim Vansa <[hidden email]> wrote:
I haven't checked Sebastian's refactored code, but does it use Optionals
as a *field* type? That's misuse (same as using it as an arg), it's
intended solely as method return type.

Radim

On 05/23/2017 05:45 PM, Katia Aresti wrote:
> Dan, I disagree with point 2 where you say "You now have a field that
> could be null, Optional.empty(), or Optional.of(something)"
>
> This is the point of optional. You shouldn't have a field that has
> these 3 possible values, just two of them = Some or None. If the field
> is mutable, it should be initialised to Optional.empty(). In the case
> of an API, Optional implicitly says that the return value can be
> empty, but when you return a "normal" object, either the user reads
> the doc, either will have bugs or boilerplate code defending from the
> possible null value (even if never ever this API will return null)
>
> :o)
>
> Cheers
>
>
>
> On Tue, May 23, 2017 at 3:58 PM, Dan Berindei <[hidden email]
> <mailto:[hidden email]>> wrote:
>
>     I wouldn't say I'm an extreme naysayer, but I do have 2 issues
>     with Optional:
>
>     1. Performance becomes harder to quantify: the allocations may or
>     may not be eliminated, and a change in one part of the code may
>     change how allocations are eliminated in a completely different
>     part of the code.
>     2. My personal opinion is it's just ugly... instead of having one
>     field that could be null or non-null, you now have a field that
>     could be null, Optional.empty(), or Optional.of(something).
>
>     Cheers
>     Dan
>
>
>
>     On Tue, May 23, 2017 at 1:54 PM, Sebastian Laskawiec
>     <[hidden email] <mailto:[hidden email]>> wrote:
>
>         Hey!
>
>         So I think we have no extreme naysayers to Optional. So let me
>         try to sum up what we have achieved so:
>
>           * In macroscale benchmark based on REST interface using
>             Optionals didn't lower the performance.
>           * +1 for using it in public APIs, especially for those using
>             functional style.
>           * Creating lots of Optional instances might add some
>             pressure on GC, so we need to be careful when using them
>             in hot code paths. In such cases it is required to run a
>             micro scale benchamark to make sure the performance didn't
>             drop. The microbenchmark should also be followed by macro
>             scale benchamrk - PerfJobAck. Also, keep an eye on Eden
>             space in such cases.
>
>         If you agree with me, and there are no hard evidence that
>         using Optional degrade performance significantly, I would like
>         to issue a pull request and put those findings into
>         contributing guide [1].
>
>         Thanks,
>         Sebastian
>
>         [1]
>         https://github.com/infinispan/infinispan/tree/master/documentation/src/main/asciidoc/contributing
>         <https://github.com/infinispan/infinispan/tree/master/documentation/src/main/asciidoc/contributing>
>
>         On Mon, May 22, 2017 at 6:36 PM Galder Zamarreño
>         <[hidden email] <mailto:[hidden email]>> wrote:
>
>             I think Sanne's right here, any differences in such large
>             scale test are hard to decipher.
>
>             Also, as mentioned in a previous email, my view on its
>             usage is same as Sanne's:
>
>             * Definitely in APIs/SPIs.
>             * Be gentle with it internals.
>
>             Cheers,
>             --
>             Galder Zamarreño
>             Infinispan, Red Hat
>
>             > On 18 May 2017, at 14:35, Sanne Grinovero
>             <[hidden email] <mailto:[hidden email]>> wrote:
>             >
>             > Hi Sebastian,
>             >
>             > sorry but I think you've been wasting time, I hope it
>             was fun :) This is not the right methodology to "settle"
>             the matter (unless you want Radim's eyes to get bloody..).
>             >
>             > Any change in such a complex system will only affect the
>             performance metrics if you're actually addressing the
>             dominant bottleneck. In some cases it might be CPU, like
>             if your system is at 90%+ CPU then it's likely that
>             reviewing the code to use less CPU would be beneficial;
>             but even that can be counter-productive, for example if
>             you're having contention caused by optimistic locking and
>             you fail to address that while making something else
>             "faster" the performance loss on the optimistic lock might
>             become asymptotic.
>             >
>             > A good reason to avoid excessive usage of Optional (and
>             *excessive* doesn't mean a couple dozen in a millions
>             lines of code..) is to not run out of eden space,
>             especially for all the code running in interpreted mode.
>             >
>             > In your case you've been benchmarking a hugely complex
>             beast, not least over REST! When running the REST Server I
>             doubt that allocation in eden is your main problem. You
>             just happened to have a couple Optionals on your path;
>             sure performance changed but there's no enough data in
>             this way to figure out what exactly happened:
>             >  - did it change at all or was it just because of a
>             lucky optimisation? (The JIT will always optimise stuff
>             differently even when re-running the same code)
>             >  - did the overall picture improve because this code
>             became much *less* slower?
>             >
>             > The real complexity in benchmarking is to accurately
>             understand why it changed; this should also tell you why
>             it didn't change more, or less..
>             >
>             > To be fair I actually agree that it's very likely that
>             C2 can make any performance penalty disappear.. that's
>             totally possible, although it's unlikely to be faster than
>             just reading the field (assuming we don't need to do
>             branching because of null-checks but C2 can optimise that
>             as well).
>             > Still this requires the code to be optimised by JIT
>             first, so it won't prevent us from creating a gazillion of
>             instances if we abuse its usage irresponsibly. Fighting
>             internal NPEs is a matter of writing better code; I'm not
>             against some "Optional" being strategically placed but I
>             believe it's much nicer for most internal code to just
>             avoid null, use "final", and initialize things aggressively.
>             >
>             > Sure use Optional where it makes sense, probably most on
>             APIs and SPIs, but please don't go overboard with it in
>             internals. That's all I said in the original debate.
>             >
>             > In case you want to benchmark the impact of Optional
>             make a JMH based microbenchmark - that's interesting to
>             see what C2 is capable of - but even so that's not going
>             to tell you much on the impact it would have to patch
>             thousands of code all around Infinispan. And it will need
>             some peer review before it can tell you anything at all ;)
>             >
>             > It's actually a very challenging topic, as we produce
>             libraries meant for "anyone to use" and don't get to set
>             the hardware specification requirements it's hard to
>             predict if we should optimise the system for this/that
>             resource consumption. Some people will have plenty of CPU
>             and have problems with us needing too much memory, some
>             others will have the opposite.. the real challenge is in
>             making internals "elastic" to such factors and adaptable
>             without making it too hard to tune.
>             >
>             > Thanks,
>             > Sanne
>             >
>             >
>             >
>             > On 18 May 2017 at 12:30, Sebastian Laskawiec
>             <[hidden email] <mailto:[hidden email]>> wrote:
>             > Hey!
>             >
>             > In our past we had a couple of discussions about whether
>             we should or should not use Optionals [1][2]. The main
>             argument against it was performance.
>             >
>             > On one hand we risk additional object allocation (the
>             Optional itself) and wrong inlining decisions taken by C2
>             compiler [3]. On the other hand we all probably "feel"
>             that both of those things shouldn't be a problem and
>             should be optimized by C2. Another argument was the
>             Optional's doesn't give us anything but as I checked, we
>             introduced nearly 80 NullPointerException bugs in two
>             years [4]. So we might consider Optional as a way of
>             fighting those things. The final argument that I've seen
>             was about lack of higher order functions which is simply
>             not true since we have #map, #filter and #flatmap
>             functions. You can do pretty amazing things with this.
>             >
>             > I decided to check the performance when refactoring REST
>             interface. I created a PR with Optionals [5], ran
>             performance tests, removed all Optionals and reran tests.
>             You will be surprised by the results [6]:
>             >
>             > Test case
>             > With Optionals [%]    Without Optionals
>             > Run 1 Run 2   Avg     Run 1   Run 2   Avg
>             > Non-TX reads 10 threads
>             > Throughput    32.54   32.87  32.71   31.74   34.04   32.89
>             > Response time -24.12  -24.63 -24.38  -24.37  -25.69  -25.03
>             > Non-TX reads 100 threads
>             > Throughput    6.48    -12.79 -3.16   -7.06   -6.14   -6.60
>             > Response time -6.15   14.93   4.39   7.88    6.49    7.19
>             > Non-TX writes 10 threads
>             > Throughput    9.21    7.60    8.41   4.66    7.15    5.91
>             > Response time -8.92   -7.11  -8.02   -5.29   -6.93   -6.11
>             > Non-TX writes 100 threads
>             > Throughput    2.53    1.65    2.09   -1.16   4.67    1.76
>             > Response time -2.13   -1.79  -1.96   0.91    -4.67   -1.88
>             >
>             > I also created JMH + Flight Recorder tests and again,
>             the results showed no evidence of slow down caused by
>             Optionals [7].
>             >
>             > Now please take those results with a grain of salt since
>             they tend to drift by a factor of +/-5% (sometimes even
>             more). But it's very clear the performance results are
>             very similar if not the same.
>             >
>             > Having those numbers at hand, do we want to have
>             Optionals in Infinispan codebase or not? And if not, let's
>             state it very clearly (and write it into contributing
>             guide), it's because we don't like them. Not because of
>             performance.
>             >
>             > Thanks,
>             > Sebastian
>             >
>             > [1]
>             http://lists.jboss.org/pipermail/infinispan-dev/2017-March/017370.html
>             <http://lists.jboss.org/pipermail/infinispan-dev/2017-March/017370.html>
>             > [2]
>             http://lists.jboss.org/pipermail/infinispan-dev/2016-August/016796.html
>             <http://lists.jboss.org/pipermail/infinispan-dev/2016-August/016796.html>
>             > [3]
>             http://vanillajava.blogspot.ro/2015/01/java-lambdas-and-low-latency.html
>             <http://vanillajava.blogspot.ro/2015/01/java-lambdas-and-low-latency.html>
>             > [4]
>             https://issues.jboss.org/issues/?jql=project%20%3D%20ISPN%20AND%20issuetype%20%3D%20Bug%20AND%20text%20%7E%20%22NullPointerException%22%20AND%20created%20%3E%3D%202015-04-27%20AND%20created%20%3C%3D%202017-04-27
>             <https://issues.jboss.org/issues/?jql=project%20%3D%20ISPN%20AND%20issuetype%20%3D%20Bug%20AND%20text%20%7E%20%22NullPointerException%22%20AND%20created%20%3E%3D%202015-04-27%20AND%20created%20%3C%3D%202017-04-27>
>             > [5] https://github.com/infinispan/infinispan/pull/5094
>             <https://github.com/infinispan/infinispan/pull/5094>
>             > [6]
>             https://docs.google.com/a/redhat.com/spreadsheets/d/1oep6Was0FfvHdqBCwpCFIqcPfJZ5-5_YYUqlRtUxEkM/edit?usp=sharing
>             <https://docs.google.com/a/redhat.com/spreadsheets/d/1oep6Was0FfvHdqBCwpCFIqcPfJZ5-5_YYUqlRtUxEkM/edit?usp=sharing>
>             > [7]
>             https://github.com/infinispan/infinispan/pull/5094#issuecomment-296970673
>             <https://github.com/infinispan/infinispan/pull/5094#issuecomment-296970673>
>             > --
>             > SEBASTIAN ŁASKAWIEC
>             > INFINISPAN DEVELOPER
>             > Red Hat EMEA
>             >
>             >
>             > _______________________________________________
>             > infinispan-dev mailing list
>             > [hidden email]
>             <mailto:[hidden email]>
>             > https://lists.jboss.org/mailman/listinfo/infinispan-dev
>             <https://lists.jboss.org/mailman/listinfo/infinispan-dev>
>             >
>             > _______________________________________________
>             > infinispan-dev mailing list
>             > [hidden email]
>             <mailto:[hidden email]>
>             > https://lists.jboss.org/mailman/listinfo/infinispan-dev
>             <https://lists.jboss.org/mailman/listinfo/infinispan-dev>
>
>
>             _______________________________________________
>             infinispan-dev mailing list
>             [hidden email]
>             <mailto:[hidden email]>
>             https://lists.jboss.org/mailman/listinfo/infinispan-dev
>             <https://lists.jboss.org/mailman/listinfo/infinispan-dev>
>
>         --
>
>         SEBASTIANŁASKAWIEC
>
>         INFINISPAN DEVELOPER
>
>         Red HatEMEA <https://www.redhat.com/>
>
>         <https://red.ht/sig>
>
>
>         _______________________________________________
>         infinispan-dev mailing list
>         [hidden email]
>         <mailto:[hidden email]>
>         https://lists.jboss.org/mailman/listinfo/infinispan-dev
>         <https://lists.jboss.org/mailman/listinfo/infinispan-dev>
>
>
>
>     _______________________________________________
>     infinispan-dev mailing list
>     [hidden email] <mailto:[hidden email]>
>     https://lists.jboss.org/mailman/listinfo/infinispan-dev
>     <https://lists.jboss.org/mailman/listinfo/infinispan-dev>
>
>
>
>
> _______________________________________________
> infinispan-dev mailing list
> [hidden email]
> https://lists.jboss.org/mailman/listinfo/infinispan-dev


--
Radim Vansa <[hidden email]>
JBoss Performance Team

_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev
--

SEBASTIAN ŁASKAWIEC

INFINISPAN DEVELOPER


_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev


_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev
Loading...