perf comments

[hexabyte23]

Just some comments after load test session on performances

1. My tests have only been be performed on macOS 10.15.4 with Instruments Time profile Apple tool, I have planned to do on Win 10 and Arch Linux soon
2. Passing String by value instead of by reference have a perf impact as expected, i had decided to transform all QString str (or other object) to const String &str in all code
3. senFile() is most used function (for my usage) and macOS implementation of mimeDatabase.mimeTypeForFile(fi, QMimeDatabase::MatchExtension).name() take a while to be executed, so i had decided to replace it by a local cache this way:

void HttpResponse::sendFile(const QString &filename, const QString &mimeType, const QString &charset, qint64 len, int compressionLevel, const QString &attachmentFilename, int cacheTime)
{
   static QMap<QString, QString> mimeTypeMap;

   QString finalMimeType = mimeType;
   QFile file(filename);
   if (!file.open(QIODevice::ReadOnly))
   {
      if (config_->verbosity >= HttpServerConfig::Verbose::Warning)
         qWarning().noquote() << QString("Unable to open file to be sent (%1): %2").arg(filename).arg(file.errorString());

      return;
   }

   if (mimeType.isEmpty())
   {
      QFileInfo fi(file);
      QString suffix = fi.suffix();
      if(mimeTypeMap.contains(suffix))
         finalMimeType = mimeTypeMap.value(suffix);
      else
      {
         finalMimeType = mimeDatabase.mimeTypeForFile(fi, QMimeDatabase::MatchExtension).name();
         mimeTypeMap[suffix] = finalMimeType;
      }
   }

   sendFile(&file, finalMimeType, charset, len, compressionLevel, attachmentFilename, cacheTime);
}

[addisonElliott]

I agree, since QString is copy-on-write, there really is no reason to pass by reference. However, I am surprised you actually found a difference in testing. I would guess this would need to be called millions of times to see a noticeable difference.

I'll be interested to see whether this is a performance issue on Windows & Linux. I would expect their to be some sort of cache in the QMimeDatabase or on the OS level itself.

I haven't done much performance testing of this library myself, so I'm glad you're taking the time to do it. In my opinion, I think this library should focus on Linux as it's primary OS since most webservers are hosted on Linux machines now-a-days. I'm not saying that performance should be terrible on OSX or Windows, but probably not the main focus. (Unless you have a use case for running a production HTTP server on OSX).

As always, PRs are welcome and I'd be happy to review them.

If you are wanting to optimize for performance, can you start by creating some sort of tests that stress test the server so we can talk of improvements in terms of numbers?

[hexabyte23]

Yes you 're right, there is only a small difference when passing QString by reference, but as always, if all Qt API are written that way, it's for a good reason.

Mac is only my dev personal platform, but yes, most of HTTP servers are Linux one, and my target OS will be Arch Linux. Xcode Instruments tool is must better than Valgrind/perf linux tools, even when code is compiled with QtCreator, this is why I keep my macOS for dev.

Yes I dev a kind of stress test socket client inside my unit testing QTest app. I iterate the code and compare results this Instruments tool between runs, so i can see real improvements. Quite difficult to PR this code as specific unit test are include. Code looks like:

void webServerStressTest(const QStringList &options)
{
   QStringList webResource;

   webResource
         << "/404.html"
         << "/blank.html"
         << "/chrono.html"
         << "/club-mg.html"
         << "/club.html"
         << "/contest-mg.html"
         << "/contest.html"
         << "/forgot-password.html"
         << "/index.html"
         << "/index2.html"
         << "/login.html"
         << "/mail-change-pass.html"
         << "/mail-reset-pass.html"
         << "/main.html"
         << "/meteo.html"
         << "/phone.html"
         << "/register.html"
         << "/settings.html"
         << "/api/v1/addPilotResult?contest_id=15&fly_time=500&distance=0.20&start_height=25"
         << "/api/v1/getRunParametersContest"
         << "/api/v1/getScoreList?contest_id=15"
         << "/api/v1/getPilotRank?contest_id=15"
         << "/api/v1/getContestList"
            ;

   QTcpSocket socket;
   socket.connectToHost(options[0], static_cast<quint16>(options[1].toInt()));
   socket.waitForConnected();
   if(socket.state() == QAbstractSocket::UnconnectedState)
   {
      qCritical() << "Web server not found" << options[0] << "port" << options[1];
      return;
   }
   qInfo() << "Connected socket to server" << options[0] << "port" << options[1];

   bool run = true;
   while(run)
   {
      int idx = QRandomGenerator::global()->bounded(0, webResource.size());
      QString url = webResource.at(idx);

      qDebug() << url;

      QByteArray req = "GET ";
      req += url.toLatin1();
      req += " HTTP/1.1\n"
             "User-Agent: Mozilla/4.0 (compatible; MSIE5.01; Windows NT)\n"
             "Host: www.rc-contest.com\n";
      req += CD_USER_COOKIE;
      req += "Accept-Language: en-us\n"
             "Accept-Encoding: gzip, deflate\n"
             "Connection: Keep-Alive\n\n";

      socket.write(req);
      socket.waitForBytesWritten();
      //currentClient_->flush();

   }
}

[addisonElliott]

Okay, I'd be happy to accept a PR for fixing the pass by reference option.

As for the performance testing, I think having a standalone test option that can run and report the number of requests per second that it supported or something.

I understand it is probably difficult to make a PR out of it but having some tests we can run over and over would be great to speeding the server up.

If I get some time, I can take a look and see what I can do about performance testing