[Qt-interest] On overriding qMalloc in QVector

[Thiago Macieira]

Em Quinta-feira 22 Outubro 2009, às 15:29:47, Benoit Jacob escreveu:
> But it seemed to be the code's intention to still allow redefining
> qMalloc, qRealloc etc: indeed there is this (a bit cryptic) comment in
> qglobal.h:
> 
> /*
>    These functions make it possible to use standard C++ functions with
>    a similar name from Qt header files (especially template classes).
> */
> Q_CORE_EXPORT void *qMalloc(size_t size);
> Q_CORE_EXPORT void qFree(void *ptr);
> Q_CORE_EXPORT void *qRealloc(void *ptr, size_t size);
> Q_CORE_EXPORT void *qMemCopy(void *dest, const void *src, size_t n);
> Q_CORE_EXPORT void *qMemSet(void *dest, int c, size_t n);
> 
> Is it indeed the intention?

The intention is to allow you to replace malloc with a different 
implementation. If you replace the system allocator, remember to override 
::operator new(size_t) as well.

> So I just wanted to bring to your attention that if your intent is
> that all these memory operations go through the above functions and
> these be overridable, then that's not the case in practice.

It doesn't override all operations. Like I said, there's new and delete too.

> So assuming that that is really the intention, here are 2 remarks:
> 
> 1) in qvector.h line 466 you have a call to ::memcpy(). Shouldn't that
> be qMemCopy?

memory copying is always memory copying. That doesn't affect in the least the 
issue at hand.

> 2) As I mentioned, only calls made from templates can be overrid,
> calls made from the binary lib can't be overrid, so in QVector the
> allocation shouldn't be performed from QVectorData (which by the way
> is independent from the type T, hence shouldn't allocate memory for
> it).

That's wrong. Overriding those functions allows you to catch all uses of them. 
Note that "overriding those functions" includes modifying qglobal.cpp and 
recompiling Qt.

As for QVectorData, if you pay attention to the code, it has special care for 
the padding that precedes T. That way, if your type has a strict alignment 
requirement, that padding will be taken care of, when calling qMalloc.

> > Qt tool classes do not support custom allocators. That feature cannot be
> > added until Qt 5, so any discussion is tabled for now.
> 
> Still you have a very simple way of fixing this without breaking
> compatibility, all what's needed is to make sure that the user can
> override qMalloc : introduce
>   QVector<T>::growData()
> containing the exact same code as QVectorData::grow(), and call that
> instead of  QVectorData::grow(). Same for QVectorData::malloc() if
> it's actually used (i don't know).

Sounds like a hack and it would be very specific to QVector. The other Qt 
classes wouldn't be changed by this.

What I meant is that we can't add a template parameter to QVector until Qt 5.

> > In any case, QVector<T>::Data contains one element of type T. I suggest
> > using compiler extensions to change the alignment of said type, like
> > GCC's __attribute__((aligned(N))).
> 
> Already done. But that doesn't affect what qMalloc does. In our case,
> whenever qMalloc returns a pointer that's not 16-byte aligned, we
> crash upon the subsequent access.

    int sizeOfTypedData() {
        // this is more or less the same as sizeof(Data), except that it 
doesn't
        // count the padding at the end
        return reinterpret_cast<const char *>(&(reinterpret_cast<const Data 
*>(this))->array[1]) - reinterpret_cast<const char *>(this);
    }

That means the alignment is taken care of by the compiler. If it isn't working 
for you, it's probably some problem in using __attribute__.

#include <QVector>
#include <QDebug>

struct __attribute__((aligned(32))) MyType { int i; };

int main()
{
    qDebug() << "Size of MyType:" << sizeof(MyType);
    qDebug() << "Size of QVectorData:" << sizeof(QVectorData);

    QVectorTypedData<MyType> *d;
    qDebug() << "Size of QVector<MyType>::Data" << sizeof(*d);

    int n = reinterpret_cast<const char *>(&d->array[0]) - 
reinterpret_cast<const char *>(d);
    qDebug() << "Size of typed data without data:" << n;
    qDebug() << "Padding added by the compiler:" << n - sizeof(QVectorData);
}

When run:
Size of MyType: 32
Size of QVectorData: 16
Size of QVector<MyType>::Data 64
Size of typed data without data: 32
Padding added by the compiler: 16

-- 
Thiago Macieira - thiago (AT) macieira.info - thiago (AT) kde.org
  Senior Product Manager - Nokia, Qt Development Frameworks
      PGP/GPG: 0x6EF45358; fingerprint:
      E067 918B B660 DBD1 105C  966C 33F5 F005 6EF4 5358
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: This is a digitally signed message part.
Url : http://lists.qt-project.org/pipermail/qt-interest-old/attachments/20091022/88d0ed8e/attachment.bin