Cool RJ :-)
It was always there but I never noticed:
http://qt-project.org/doc/qt-4.8/qstring.html#more-efficient-string-construction
Let us use this!
2013/5/1 RJ Ryan <[email protected]>
>
>
>
> On Wed, May 1, 2013 at 4:03 PM, Daniel Schürmann <[email protected]>wrote:
>
>> Hi,
>>
>> the typo resistance is an other good point for using constants.
>>
>> ---
>>
>> The performance win does not happen when issue a query, it happens when
>> starting Mixxx.
>>
>> const QString LIBRARYTABLE_ID = "id";
>>
>> "id" is in text segment after starting Mixxx. The QString constructor
>> converts the string to 16 bit unicode and stores it on the heap after
>> malloc.
>>
>> const char* = "id";
>>
>> is simply a pointer to the text segment.
>>
>
> Ah, yes -- I agree there is a cost (again, tiny) during global
> initialization. I was referring to the cost of constructing the SQL query
> in Steven's example though.
>
> #define PLAYLIST_TABLE "PlaylistTracks"
>
> QString sqlQuery = QString("SELECT FROM " PLAYLIST_TABLE " ...");
>
> This is 1 malloc and no concatenation. The compiler joins the multiple
> string literals together into one string literal.
>
> Whereas
>
> const QString PLAYLIST_TABLE = "PlaylistTracks";
>
> QString sqlQuery = QString("SELECT FROM " + PLAYLIST_TABLE + "...");
>
> Involves potentially 2 mallocs before the outer QString constructor is
> called (the outer QString will probably use Qt's implicit sharing so no
> malloc will happen).
>
> Since often in the DAOs very long queries are constructed by strings of +
> operators and QString string interpolation, there is a whole lot of malloc
> and copy going on every time we want to format a query.
>
> I still say it's negligible compared to any real work going on -- but
> relative to global static initialization (a one time cost at bootup on the
> order of tens of microseconds, max) I would say this is much more expensive
> since it happens every time we construct a query and often does a lot more
> work than a single malloc / memcpy.
>
> I read a post a while back about QStringBuilder. In Qt 4.6 you can
> basically define QT_USE_QSTRINGBUILDER and it converts QString's operator +
> to use lazy evaluation. We may as well turn that on?
>
> http://blog.qt.digia.com/blog/2011/06/13/string-concatenation-with-qstringbuilder/
>
>
>
>>
>> I am also not sure if putting this to trackdao.h will duplicate the
>> sting in text segment each time trackdao.h is included. So we should make
>> them static members of TrackDAO to be sure.
>>
>
> The QString definitions will appear in the data section of every .o that
> includes trackdao.h. Ideally the linker would collapse the duplicates but
> I'm guessing it doesn't :). Someone could easily check by running 'strings'
> on the mixxx binary.
>
>
>> Just two cents from an embedded developer ;-)
>>
>> Kind regards,
>>
>> Daniel
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> 2013/5/1 RJ Ryan <[email protected]>
>>
>>> The main reason for me is that they are a protection measure against
>>> making typos in SQL queries. You'd think a typo would get noticed but not
>>> all queries fail immediately when they have a mistake in them. Some of them
>>> just return no results, or return wrong results. For queries that are off
>>> the beaten path, we end up with issues like
>>> this<https://bugs.launchpad.net/mixxx/+bug/1175024>(which Steven just
>>> fixed).
>>>
>>> In fact, I would prefer we totally got rid of writing SQL queries
>>> directly in code and instead composed them using declarative C++ classes
>>> which are then compiled down to SQL. Not quite a full-on ORM, just
>>> something for composing SQL queries (like SQLAlchemy for C++).
>>>
>>>
>>> On Wed, May 1, 2013 at 2:56 PM, Owen Williams <[email protected]>wrote:
>>>
>>>> Are the constants really a good idea? What is the purpose behind using
>>>> them? Ideally the schema shouldn't be changing very often so having
>>>> them hardcoded seems ok from a maintenance perspective. And from a
>>>> readability perspective, the constant method looks really ugly to me.
>>>> I'm just wondering what the other side of the argument is.
>>>>
>>>> owen
>>>>
>>>> On Wed, 2013-05-01 at 14:42 -0400, RJ Ryan wrote:
>>>> > On Wed, May 1, 2013 at 2:15 PM, Daniel Schürmann <[email protected]>
>>>> > wrote:
>>>> > Hi Steven,
>>>> >
>>>> > Yes, we should solve this issue and use the const values in
>>>> > any case.
>>>> > This allows to rely on IDE features like "display call tree".
>>>> >
>>>> >
>>>> >
>>>> > I think we should stick with whatever makes the code easiest to
>>>> > maintain (which would be using the constants when possible) -- maybe
>>>> > the constants should be renamed to be a little easier on the eyes? A
>>>> > lot of the DAO code doesn't use the constants, that's true. We should
>>>> > fix those queries.
>>>> >
>>>> >
>>>> > But I am not sure if the use of const QString is performance
>>>> > neutral to const char* and #define because the heavy QSting
>>>> > constructor is called.
>>>> >
>>>> > So IMHO we should change it to const char*.
>>>> >
>>>> >
>>>> > There are a lot of non-performance-neutral choices we make in Mixxx,
>>>> > but that alone isn't enough to make a decision, we have to take the
>>>> > context into account. Compared to the QSqlQuery::exec() that comes
>>>> > immediately after all these queries, some string concatenation isn't
>>>> > going to break the bank, cycle-wise. Processors are pretty good at
>>>> > memcpy these days :).
>>>> >
>>>> >
>>>> > Kind regards,
>>>> >
>>>> > Daniel
>>>> >
>>>> >
>>>> >
>>>> >
>>>> >
>>>> > Am 01.05.2013 17:32, schrieb Steven Boswell II:
>>>> >
>>>> > > Most of the DAO header files in src/library/dao have
>>>> > > definitions for the names of tables, and the names of
>>>> > > columns in the tables, but they're used sporadically; quite
>>>> > > often, references to them are hardcoded.
>>>> > >
>>>> > >
>>>> > > I'm writing a new feature (the aforementioned
>>>> > > auto-DJ-crates), and the necessary info is stored in a
>>>> > > temporary table, so I'm writing several SQL queries. I'm
>>>> > > just wondering if I should hardcode the table/column
>>>> > > references (which won't automatically get updated if the
>>>> > > underlying names change), or try to write them using the
>>>> > > table/column names defined in header files (which makes the
>>>> > > query harder to read).
>>>> > >
>>>> > >
>>>> > > Here's an example. The comment is the hardcoded,
>>>> > > human-readable version, and following it is the hard-to-read
>>>> > > one that'll adapt automatically to changes.
>>>> > >
>>>> > >
>>>> > >
>>>> > > // INSERT INTO temp_autodj_crates (track_id, craterefs,
>>>> > > timesplayed, autodjrefs) SELECT crate_tracks.track_id, COUNT
>>>> > > (*), library.timesplayed, 0 FROM crate_tracks, library WHERE
>>>> > > crate_tracks.crate_id IN (SELECT id FROM crates WHERE autodj
>>>> > > = 1) AND crate_tracks.track_id = library.id AND
>>>> > > library.mixxx_deleted = 0 GROUP BY crate_tracks.track_id,
>>>> > > library.timesplayed;
>>>> > > strQuery = QString ("INSERT INTO " AUTODJCRATES_TABLE
>>>> > > " (" AUTODJCRATESTABLE_TRACKID ", "
>>>> > > AUTODJCRATESTABLE_CRATEREFS ", "
>>>> > > AUTODJCRATESTABLE_TIMESPLAYED ", "
>>>> > > AUTODJCRATESTABLE_AUTODJREFS ")"
>>>> > > " SELECT " CRATE_TRACKS_TABLE ".%1 , COUNT (*), "
>>>> > > LIBRARY_TABLE ".%2, 0 FROM " CRATE_TRACKS_TABLE ", "
>>>> > > LIBRARY_TABLE
>>>> > > " WHERE " CRATE_TRACKS_TABLE ".%4 IN "
>>>> > > "(SELECT %5 FROM " CRATE_TABLE " WHERE %6 = 1) AND "
>>>> > > CRATE_TRACKS_TABLE ".%1 = " LIBRARY_TABLE ".%7 AND "
>>>> > > LIBRARY_TABLE
>>>> > > ".%3 == 0 GROUP BY " CRATE_TRACKS_TABLE ".%1, "
>>>> > > LIBRARY_TABLE ".%2")
>>>> > > .arg (CRATETRACKSTABLE_TRACKID) // %1
>>>> > > .arg (LIBRARYTABLE_TIMESPLAYED) // %2
>>>> > > .arg (LIBRARYTABLE_MIXXXDELETED) // %3
>>>> > > .arg (CRATETRACKSTABLE_CRATEID) // %4
>>>> > > .arg (CRATETABLE_ID) // %5
>>>> > > .arg (CRATETABLE_AUTODJ) // %6
>>>> > > .arg (LIBRARYTABLE_ID); // %7
>>>> > >
>>>> > >
>>>> > >
>>>> > > Which should I do?
>>>> > >
>>>> > >
>>>> > > Pretty soon, I'll make a blueprint for the auto-DJ-crates
>>>> > > feature.
>>>> > >
>>>> > >
>>>> > >
>>>> > > Steven Boswell
>>>> > >
>>>> > >
>>>> > >
>>>> > >
>>>> > >
>>>> ------------------------------------------------------------------------------
>>>> > > Introducing AppDynamics Lite, a free troubleshooting tool
>>>> for Java/.NET
>>>> > > Get 100% visibility into your production application - at
>>>> no cost.
>>>> > > Code-level diagnostics for performance bottlenecks with <2%
>>>> overhead
>>>> > > Download for free and get started troubleshooting in
>>>> minutes.
>>>> > > http://p.sf.net/sfu/appdyn_d2d_ap1
>>>> > >
>>>> > >
>>>> > > _______________________________________________
>>>> > > Get Mixxx, the #1 Free MP3 DJ Mixing software Today
>>>> > > http://mixxx.org
>>>> > >
>>>> > >
>>>> > > Mixxx-devel mailing list
>>>> > > [email protected]
>>>> > > https://lists.sourceforge.net/lists/listinfo/mixxx-devel
>>>> >
>>>> >
>>>> >
>>>> >
>>>> ------------------------------------------------------------------------------
>>>> > Introducing AppDynamics Lite, a free troubleshooting tool for
>>>> > Java/.NET
>>>> > Get 100% visibility into your production application - at no
>>>> > cost.
>>>> > Code-level diagnostics for performance bottlenecks with <2%
>>>> > overhead
>>>> > Download for free and get started troubleshooting in minutes.
>>>> > http://p.sf.net/sfu/appdyn_d2d_ap1
>>>> > _______________________________________________
>>>> > Get Mixxx, the #1 Free MP3 DJ Mixing software Today
>>>> > http://mixxx.org
>>>> >
>>>> >
>>>> > Mixxx-devel mailing list
>>>> > [email protected]
>>>> > https://lists.sourceforge.net/lists/listinfo/mixxx-devel
>>>> >
>>>> >
>>>> >
>>>> ------------------------------------------------------------------------------
>>>> > Introducing AppDynamics Lite, a free troubleshooting tool for
>>>> Java/.NET
>>>> > Get 100% visibility into your production application - at no cost.
>>>> > Code-level diagnostics for performance bottlenecks with <2% overhead
>>>> > Download for free and get started troubleshooting in minutes.
>>>> > http://p.sf.net/sfu/appdyn_d2d_ap1
>>>> > _______________________________________________ Get Mixxx, the #1
>>>> Free MP3 DJ Mixing software Today http://mixxx.org Mixxx-devel mailing
>>>> list [email protected]
>>>> https://lists.sourceforge.net/lists/listinfo/mixxx-devel
>>>>
>>>>
>>>>
>>>
>>
>
------------------------------------------------------------------------------
Introducing AppDynamics Lite, a free troubleshooting tool for Java/.NET
Get 100% visibility into your production application - at no cost.
Code-level diagnostics for performance bottlenecks with <2% overhead
Download for free and get started troubleshooting in minutes.
http://p.sf.net/sfu/appdyn_d2d_ap1
_______________________________________________
Get Mixxx, the #1 Free MP3 DJ Mixing software Today
http://mixxx.org
Mixxx-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/mixxx-devel
