Caching issue when P1.memo is null

[mperry]

See the comment by @zot at #106 (comment).

[zot]

I'm not positive that the generated bytecode invokes the constructor before the assignment to v. I THINK this code is safe in the case that it invokes the constructor after the assignment (since the first part of the code doesn't use synchronization):

public A _1() {
  if (v != null) return v.get();
  synchronized (latch) {
    if (v != null) return v.get(); //check again here in case v was assigned asynchronously
    A a = self._1();
    SoftReference<A> tmp = new SoftReference<A>(a);
    v = tmp;
    return a;
  }
}

[gneuvill]

What do you guys think of https://gist.github.com/gneuvill/d5db376a266bf3af004c ?

I discern the following advantages of it :

• null safety
• efficiency (neither locks nor synchronization)

There is one drawback though, but I can't rule on its severity : the memoized P1 may be evaluated more than once in a highly concurrent environment. But the test figuring in the gist seems to show that the number of evaluation stays negligible in comparison with the number of call to _1() (for example, 2 or 3 evaluation for 10.000.000 calls on my machine). That concern is obviously non existent in a sequential context.

P.S : the code of the memo method is copy/pasted from AtomicReference.updateAndGet available from java 8 on.

[jbgi]

I am not sure I understand the rational for using SoftReference in all the implementations I have seen. Looks to me that this may lead to calling the original p1 multiple times (not what the doc says).
What about using this idiom:
https://github.com/javafp/functionaljava/commit/42cba51748c042ffaeb373d27be9185343ab90a5
?
(adapted from https://github.com/google/guava/blob/master/guava/src/com/google/common/base/Suppliers.java)

[zot]

Why the extra a variable and return statement? Isn't OK?

@OverRide
public A _1() {
// A 2-field variant of Double Checked Locking.
if (delegate != null) {
synchronized (this) {
if (delegate != null) {
value = delegate.f();
delegate = null;
}
}
}
return value;
}

On Sun, Apr 12, 2015 at 3:01 AM, JB Giraudeau notifications@github.com
wrote:

I am not sure I understand the rational for using SoftReference in all the
implementations I have seen. Looks to me that this may lead to calling the
original p1 multiple times (not what the doc says).
What about using this idiom:
javafp@42cba51
https://github.com/javafp/functionaljava/commit/42cba51748c042ffaeb373d27be9185343ab90a5
?
(adapted from
https://github.com/google/guava/blob/master/guava/src/com/google/common/base/Suppliers.java
)

—
Reply to this email directly or view it on GitHub
#108 (comment)
.

[jbgi]

This is because access to local variable is a tiny bit faster than access to instance variable.

[zot]

I think readability is more important in this case but besides that there
are some issues that might make this type of micro-optimization not so
effective or beneficial.

Is accessing a local variable really a tiny bit faster than accessing an
instance variable?

There is also an extra local variable assignment -- is a local variable
assignment + a local variable access faster than an instance variable
access?

It's a trick. My questions are actually red herrings. Questions about
what types of bytecode assignment instructions are faster are really
impossible to evaluate because virtual machines are so irritatingly
virtual. Context matters, too -- a variable access might not even happen,
depending on the situation. Debuggers protect us from this ephemeral world
and lead us to believe that we know what's actually happening when we
really may not.

Deployments will usually use bytecode optimization (ProGuard, DexGuard),
JIT compilation (HotSpot), or AOT compilation (Android 5.0's ART), and they
may use a combination. For instance, a low-power deployment environment
that can't afford a JIT (like Android) might use bytecode optimization
and/or AOT compilation. Bytecode optimization is a tricky thing because it
could happen at compilation time, at build time (like if you're making an
APK), at load time, or even at JIT-time (a JIT compiler might use a
bytecode optimizer as part of its pipeline).

Bytecode optimization might eliminate a value access in {value = x ...
return value} by using a DUP instruction.

JIT or AOT compilation that uses registers for return values might just
leave the return value from delegate.f() in the register and eliminate the
instance variable access.

-- Bill

On Sun, Apr 12, 2015 at 1:43 PM, JB Giraudeau notifications@github.com
wrote:

This is because access to local variable is a tiny bit faster than access
to instance variable.

—
Reply to this email directly or view it on GitHub
#108 (comment)
.

[jbgi]

according to http://stackoverflow.com/questions/21613098/java-local-vs-instance-variable-access-speed it is 'almost' 1% faster. I agree that the micro-optimization is effectively almost useless because the value was just written and is probably still in cache. I agree that in this case readability should prime.
(the reason I used it is mostly because I copy-pasted from guava)

[jbgi]

But I am more concerned over the use of SoftwareReference, are they necessary because of the use of memo() in the Stream implementation? in which case my proposition may cause a memory retention in some cases.

[jbgi]

@zot by the way, in your #108 (comment) you were also using a temporary variable 'a' and extra returns. ie you could have simply:

public A _1() {
  if (v == null) {
    synchronized (latch) {
      if (v == null) { //check again here in case v was assigned asynchronously
        SoftReference<A> tmp = new SoftReference<A>(self._1());
        v = tmp;
      }
    }
  }
  return v.get()
}

In any case your proposition could still cause null-pointers because the reference in v could have been garbage collected.

[zot]

Yeah, that was unnecessary. I think it was from a quick code
transformation I did and failed to clean up but it was not for optimization.

On Mon, Apr 13, 2015 at 3:05 PM, JB Giraudeau notifications@github.com
wrote:

@zot https://github.com/zot by the way, in your #108 (comment)
#108 (comment)
you were also using a temporary variable 'a' and extra returns. ie you
could have simply:

public A _1() {
if (v == null) {
synchronized (latch) {
if (v == null) { //check again here in case v was assigned asynchronously
SoftReference tmp = new SoftReference(self._1());
v = tmp;
}
}
}
return v.get()
}

In any case your proposition could still cause null-pointers because the
reference in v could have been garbage collected.

—
Reply to this email directly or view it on GitHub
#108 (comment)
.

[jbgi]

@gneuvill What if the SoftReference is garbaged? In your solution it looks like _1() will always return null then.

[mperry]

After considering the discussion here I created PR #141 which puts the possibly null value inside a softly referenced Option. This is equivalent to the solution from @jbgi in #127, but is closer to the original code.