hg: lambda/lambda/jdk: - ensure null values are retained

[Paul Sandoz]

Hi Remi,

Many thanks, that is just the kind of VM-perspective advice i was looking for. I was assuming that the VM would be able to somehow inline the hot path to the else block.

So you are suggesting something like this:

                    T _t = (T) maskNull(t);
                    if (lastSeen == null || !lastSeen.equals(_t)) {
                        lastSeen = _t;
                        downstream.accept(t);
                    }

rather than:

                    if (t == null) {
                        if (!seenNull) {
                            seenNull = true;
                            downstream.accept(lastSeen = null);
                        }
                    } else if (lastSeen == null || !t.equals(lastSeen)) {
                        downstream.accept(lastSeen = t);
                    }

Paul.

On Dec 4, 2012, at 9:25 AM, Remi Forax <forax at univ-mlv.fr> wrote:

> On 12/03/2012 04:59 PM, paul.sandoz at oracle.com wrote:
>> Changeset: 8d6fc90e7bbf
>> Author:    psandoz
>> Date:      2012-12-03 16:57 +0100
>> URL:       http://hg.openjdk.java.net/lambda/lambda/jdk/rev/8d6fc90e7bbf
>> 
>> - ensure null values are retained
>> - add very simple data provider for combination of numbers and null values
>> 
>> ! src/share/classes/java/util/stream/op/UniqOp.java
>> ! test-ng/tests/org/openjdk/tests/java/util/stream/StreamTestDataProvider.java
>> ! test-ng/tests/org/openjdk/tests/java/util/stream/op/UniqOpTest.java
>> 
>> 
> 
> Hi Paul,
> that's not the good way(TM) to handle null.
> 
> A better approach is to use the Null Object design pattern, i.e.
> having a constant final field Object with a known object and each time
> a null is sent, null is replaced by the Object and each time the element
> need to be sent, if element value is equals to the known Object, null is 
> sent.
> You can take a look to IdentityHashMap.maskNull/unmaskNull [1] which already
> implements this pattern.
> 
> This can be done locally to each ops or globally for the whole pipeline,
> the former solution is enough in my opinion.
> 
> The null object pattern is better because it adds only two checks on 
> variable that are already in register
> if the code never see a null (the usual case). Your solution require to 
> load/store value from the RAM
> because the VM will not able to optimize this code (inlining is often 
> defeated due to the fact that the call
> between ops is a virtual call).
> 
> cheers,
> Rémi
> 
> [1] 
> http://hg.openjdk.java.net/jdk8/jdk8/jdk/file/df5619994dc3/src/share/classes/java/util/IdentityHashMap.java 
> 
>