Re: Slow default methods in abstract classes

[Jukka Zitting]

Hi,

On Thu, Feb 6, 2014 at 4:45 AM, Thomas Mueller  wrote:
> Because we forgot(?) to override those methods, we sometimes ended up with
> very slow performance.

The original purpose of the abstract classes and the generic,
unoptimized methods in them was to simplify experimentation and
initial implementations when the NodeStore API was being developed.
Nowadays they are mostly useful for correct handling of corner cases
for which no better/faster implementation is available, but you're
right in that it's often too easy to forget that a better
implementation is almost always needed for the normal use cases.

> If a "convenience" implementation is useful, but slow, it should have
> a different name, for example "equalsBruteForce" or
> AbstractNodeState.compareAgainstBaseStateBruteForce(..).

Sounds good. We still do need those methods for the purpose mentioned
above, but naming them more explicitly (or having them only as static
utility methods like we already do for many AbstractNodeState methods)
makes sense.

BR,

Jukka Zitting

Slow default methods in abstract classes

[Thomas Mueller]

Hi,

We have some abstract classes that implement some methods for convenience.
For example, AbstractBlob.equals() is implemented by comparing all bytes
of a blob, which is very slow (OAK-1392). Or
AbstractNodeState.compareAgainstBaseState.

Because we forgot(?) to override those methods, we sometimes ended up with
very slow performance.

I would prefer if the implementations in the abstract classes are not
slow. Instead, I would prefer if such methods would be abstract or not
implemented (in the case of equals). If a "convenience" implementation is
useful, but slow, it should have a different name, for example
"equalsBruteForce" or
AbstractNodeState.compareAgainstBaseStateBruteForce(..).

That way, a non-abstract implementation could still use the default (slow)
implementation (for example for the in-memory case, or if the amount of
data is known to be small), but it would have to do that explicitly.

Regards,
Thomas