Skip to content

Static methods of Transaction could return QStringView instead of allocating a new QString #55

@ratijas

Description

@ratijas

All of the following methods do not modify the input string, and neither do they construct a new string from any concatenations that would justify the allocation of a new QString:

src/transaction.h:

    /**
     * Returns the package name from the \p packageID
     */
    static QString packageName(const QString &packageID);

    /**
     * Returns the package version from the \p packageID
     */
    static QString packageVersion(const QString &packageID);

    /**
     * Returns the package arch from the \p packageID
     */
    static QString packageArch(const QString &packageID);

    /**
     * Returns the package data from the \p packageID
     */
    static QString packageData(const QString &packageID);

In KDE/Plasma Discover there is a copy-pasta of packageName method for the sake of performance improvement. Here's a copy of that comment of top:

// Copy of Transaction::packageName that doesn't create a copy but just pass a reference
// It's an optimisation as there's a bunch of allocations that happen from packageName
// Having packageName return a QStringRef or a QStringView would fix this issue.
// TODO Qt 6: Have packageName and similars return a QStringView

Changing it in packagekit-qt now would be an ABI brakage, and you can't overload by return types in C++. But it should at least still be mostly API-compatible, as QStringView implicitly converts to QString when needed.

Alternatively, we could overloaded counterparts for all those methods which both take and return a string view. That would require explicit porting in apps, if they want to take advantage of performance gains.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions