free is always an override in classes

[davidhesselbom]

For any class in other than Object, this is a valid implementation of free in a class:

free: override func {
    [clean up your mess]
    super()
}

This is not:

free: func {
    [clean up your mess]
}

and neither is this:

free: override func {
    [clean up your mess]
    // no super
}

Also, this is useless:

free: override func {
    super()
}

Magic should be able to detect when a class has an invalid free function, but it will require that magic knows it's looking at a class, and not a cover, because those rules don't apply there.

[thomasfanell]

Why is free: override func { super() } useless?

[davidhesselbom]

Because you can remove it entirely and get the exact same result - the parent's implementation will be used instead (that's how override works).

[davidhesselbom]

Any ETA on this?

[simonmika]

Note that not all code paths have to do the super call.

[davidhesselbom]

Can you name any object, except BufferedPlayer (which needs refactoring), that doesn't?

[ghost]

Derived class destructor that does not have to call the base class destructor ? This is not even possible in C++ :)

[simonmika]

https://github.com/cogneco/ooc-kean/blob/master/source/base/ByteBuffer.ooc#L90
https://github.com/cogneco/ooc-kean/blob/master/source/base/ByteBuffer.ooc#L101

[simonmika]

Free might not always deallocate the object. That might be done in other ways.

[davidhesselbom]

Okay, fine.

[ghost]

I think it could be easier if RecyclableByteBuffer and friends were actually a lightweight wrappers around the single buffer implementation. Recoverable etc. variants could put back the internal representation into the global collection and call base class destructor. Now anyone wanting a real (not reusable) ByteBuffer is forced to use reusable variants anyway. This leads to hacks like the forceFree flag. I think client classes (like raster images) should state explicitly if they want reusable variants or not, because now calling ByteBuffer new does not give you the ByteBuffer object that you can simply get rid of. This makes it hard to use ByteBuffer if you know what you are doing.
This feels kinda like having a garbage collector around ptr = malloc and free(ptr) and being forced to call force_free(ptr) to release memory instead because your free(ptr) does not really free.
I'm thinking about separating the buffer and reusability into two diferent layers, now everything is mixed up in the same layer of abstraction. This would allow clients to use the basic buffer building block if desired without the recycle bin logic attached.

tl:dr: make a basic buffer "building block", change various buffer classes into "interfaces" that use composition to manage the ByteBuffer instance that gets put back into global bin or deleted etc. depending on the logic in the "interface" object, and make the interface to get various versions explicit.

OK, so this is what I think about it. I don't expect you to agree. In fact I'm pretty sure you won't :) But really, the fact that you can omit the base class destructor in derived is kinda scary. Does this come from C# as well ? I think it is even worse than throwing exceptions from destructors :/

[simonmika]

I agree that forceFree seams to be some kind of hack. It leaks the implementation.

If one needs to have a ByteBuffer without recycling and manually calling malloc all one needs to do is add another constructor as it seams to be missing at the moment.

It is however not a very good abstraction around malloc, free, memset, etc. This as it uses a ReferenceCounter as well.

I don't agree on the other hand that it is wrong to use free in the way it is used here. There might be benefits of using multiple objects that is also the main drawback. Finally I definitely don't agree that the reusability is mixed up with the other buffers. It is clearly in one class at the very bottom of the file.

In the end I just would like to remind you that in ooc neither new nor free/delete are operators. They are just methods like other methods. Throwing exceptions from destructors in C++ leaks memory while this does not do that. In the end I think it is your C++ reflexes that are kicking in. I should add that in C# one has to implement it the way you suggest but in ooc it is not only possible but also encouraged in the documentation: (from https://ooc-lang.org/docs/lang/classes/)

Dog: class {
  pool := static Stack<This> new()

  new: static func -> This {
    if (pool empty?()) {
      obj := This alloc()
      obj __defaults__()
      obj
    } else {
      pool pop()
    }
  }

  free: func {
    pool push(this)
  }
}

But in the end I must just state for the record that I really dislike the forceFree construction and the flag it comes with.

[ghost]

Yes it is completely different design and thinking in ooc. Not worse or better, just different.

[davidhesselbom]

I really dislike the forceFree construction and the flag it comes with.

Hence https://github.com/cogneco/ooc-kean/issues/698...

[ghost]

Finally I definitely don't agree that the reusability is mixed up with the other buffers. It is clearly in one class at the very bottom of the file.

Yes, but I meant that it can be more loosely coupled, by moving the recoverable logic to separate file and make it possible to reuse for other classes, like

// Recyclable.ooc

Recyclable: class <T> {
    _lock := static Mutex new()
    _smallRecycleBin := static VectorList<T> new()
    _object: T = null
    ...
    new: static func ~fromSize (size: Int) -> This {
        bin := This _getBin(size)
        This _lock lock()
        for (i in 0 .. bin count) {
            if ((bin[i] size) == size) {
                                // search for free object in bin and store in 'this _object '
            }
        }
        This _lock unlock()
        this _object
    }
    free: override func {
        if (this _object)
           putToBin(this _object)
        super()
    }
}

// etc. for other reusable types 

Then you could have the reusable logic applied to any class, like

buffer := ByteBuffer new(100) // ordinary byte buffer
buffer := Recyclable<ByteBuffer> new(100) // reusable one

vector := VectorList<Float> new()
vector := Recyclable<VectorList<Float>> new()

matrix := FloatMatrix new()
matrix := Recyclable<FloatMatrix> new()

// etc ...

I don't know the details of implementation yet, this is just an idea.

Btw. in ooc it is easier to implement thanks to the overriden static new, Recyclable<T> new() can return instances of T. In C++ I've implemented something similar with a template trickery like

template<typename T>
class Reusable : public T {
};

The so called "curiously recurring template pattern" (a kind of). I wonder it it's even possible in ooc

Test: class <T> extends T {
}

Anyway, now we have ReusableByteBuffer, I think we could have both Reusable and ByteBuffer.

[simonmika]

We did do something in this direction in https://github.com/cogneco/Kean/tree/develop/Kean/Recycle.
What you suggest here is an interesting concept.