On Sat, Jul 27, 2019 at 4:38 PM Uwe L. Korn <uw...@xhochy.com> wrote:
>
> The PIMPL is a thing I would trade a bit of performance as it brings ABI
> stability. This is something that will help us making Arrow usage in
> thirdparty code much simpler.
>
I question whether ABI stability (at the level of the shared library
ABI version) will ever be practical in this project. In the case of
NumPy, there are very few C data structures that could actually
change. In this library we have a great deal more data structures, I
would guess 100s of distinct objects taking into account each C++
class in the library. It's one thing to be API forward compatible from
major version to major version (with warnings to allow for graceful
deprecations) but such forward compatibility strategies are not so
readily available when talking about the composition of C++ classes
(considering that virtual function tables are part of the ABI).
In any case, as a result of this, I'm not comfortable basing technical
decisions on the basis of ABI stability considerations.
> Simple updates when an API was only extended but the ABI is intact is a great
> ease on the Arrow consumer side. I know that this is a bit more hassle on the
> developer side but it's something I really love with NumPy. It's so much
> simpler to do a version upgrade than with an ABI breaking library such as
> Arrow.
>
> Uwe
>
> > Am 27.07.2019 um 22:57 schrieb Jed Brown <j...@jedbrown.org>:
> >
> > Wes McKinney <wesmck...@gmail.com> writes:
> >
> >> The abstract/all-virtual base has some benefits:
> >>
> >> * No need to implement "forwarding" methods to the private implementation
> >> * Do not have to declare "friend" classes in the header for some cases
> >> where other classes need to access the methods of a private
> >> implementation
> >> * Implementation symbols do not need to be exported in DLLs with an
> >> *_EXPORT macro
> >>
> >> There are some drawbacks, or cases where this method cannot be applied,
> >> though:
> >>
> >> * An implementation of some other abstract interface which needs to
> >> appear in a public header may not be able to use this approach.
> >> * My understanding is that the PIMPL pattern will perform better for
> >> non-virtual functions that are called a lot. It'd be helpful to know
> >> the magnitude of the performance difference
> >> * Complex inheritance patterns may require use of virtual inheritance,
> >> which can create a burden for downstream users (e.g. they may have to
> >> use dynamic_cast to convert between types in the class hierarchy)
> >
> > I would add these two points, which may or may not be a significant
> > concern to you:
> >
> > * When you add new methods to the abstract virtual model, you change the
> > ABI [1] and therefore need to recompile client code. This has many
> > consequences for distribution.
> >
> > * PIMPL gives a well-defined place for input validation and setting
> > debugger breakpoints even when you don't know which implementation
> > will be used.
> >
> >
> > [1] The ABI changes because code to index into the vtable is inlined at
> > the call site. Adding to your example
> >
> > void foo(VirtualType &obj) {
> > obj.Method1();
> > // any other line to suppress tail call
> > }
> >
> > produces assembly like
> >
> > mov rax,QWORD PTR [rdi] ; load vtable for obj
> > call QWORD PTR [rax+0x10] ; 0x10 is offset into vtable
> >
> > A different method will use a different offset. If you add a method,
> > offsets of existing methods may change. With PIMPL, the equivalent
> > indexing code resides in your library instead of client code, and yields
> > a static (or PLT) call resolved by the linker:
> >
> > call _ZN9PIMPLType7Method1Ev@PLT
>
