Question about `aspl::Device` custom implementation

[fine-00]

Hi, @gavv

First of all, thank you so much for creating this library. The code is very clean, organized, and easy to follow. Also, the default implementations are helping me so much.

I have a question about usage of the library.

I am trying to create a custom device by inheriting aspl::Device. (i.e. class CustomDevice : public aspl::Device { ... };)

While doing so, I figured that overriding GetZeroTimeStamp() (and some other virtual methods) is near unfeasible due to ioMutex_ being a private member and there seems to be no way of locking I/O operation from subclasses.

Is this intended or am I doing something wrong? (perhaps custom implementation of GetZeroTimeStamp() is not intended?)

Thank you very much in advance.

[gchilds]

You can replace GetZeroTimeStamp(), you just can't use the ioMutex_.

[fine-00]

Does that mean I don't need mutex for GetZeroTimeStamp() when replacing it?

I'm starting to wonder why it needed a mutex in the first place.

[gavv]

Hi, thanks for feedback and report and sorry for delay!

Is this intended or am I doing something wrong? (perhaps custom implementation of GetZeroTimeStamp() is not intended?)

You're not doing anything wrong, this part just not designed well enough to cover your case.

I've pushed a commit: 90e83424fd1174ef62e342726938b91520390db4

That introduces a set of new protected overridable methods: StartIOImpl(), StopIOImpl(), GetZeroTimeStampImpl(), WillDoIOOperationImpl(), BeginIOOperationImpl(), DoIOOperationImpl(), EndIOOperationImpl().

They're invoked after doing all housekeeping (tracing, argument checks) and under a lock, so that you can replace or decorate behavior without touching boilerplate and locking.

The old methods (without "Impl") are still there too, but if you override one, most likely you want to override all of them together and implement your own locking. I kept them virtual for the case if someone needs to handle I/O completely differently, and for compatibility.

---

Does that mean I don't need mutex for GetZeroTimeStamp() when replacing it? I'm starting to wonder why it needed a mutex in the first place.

If I remember correctly, in my experiments I saw that StartIO and StopIO may be called from different thread compared to the rest I/O operations. But I'm not 100% sure now.

This things are poorly documented and may change, so I prefer defensive programming tactics here. The mentioned methods have shared state, thus the state is protected by a mutex. On hot path there is no contention on that mutex, so the performance hit is negligible I think.

---

I'm going to test the new changes in my projects that use libASPL, and if everything is ok, I'll tag a release.

[gavv]

Seems everything works fine, I've tagged 3.1.0: https://github.com/gavv/libASPL/releases/tag/v3.1.0

Let me know if this approach works for your case.

[gavv]

Closing as complete, feel free to reopen if needed.