[prev in list] [next in list] [prev in thread] [next in thread]
List: kde-commits
Subject: branches/KDE/4.1/kdeedu/marble
From: Jens-Michael Hoffmann <jensmh () gmx ! de>
Date: 2008-07-19 20:31:29
Message-ID: 1216499489.460542.7288.nullmailer () svn ! kde ! org
[Download RAW message or body]
SVN commit 835029 by jmhoffmann:
Fix bug 162681 in trunk (slow tile download).
The problem with the download implementation was that
there was only one QHttp object in HttpFetchFile, throught
which all requests were serialized.
Now we have got one QHttp object per active HttpJob, so
this bottleneck is gone now.
This fix was committed in trunk earlier this day.
CCBUG: 162681
BUG: 162681
M +14 -0 ChangeLog
M +3 -3 src/lib/HttpDownloadManager.cpp
M +54 -35 src/lib/HttpFetchFile.cpp
M +13 -2 src/lib/HttpFetchFile.h
--- branches/KDE/4.1/kdeedu/marble/ChangeLog #835028:835029
@@ -1,3 +1,17 @@
+2008-07-19 Jens-Michael Hoffmann <jensmh@gmx.de>
+
+ * src/lib/HttpFetchFile.cpp:
+ * src/lib/HttpFetchFile.h:
+ * src/lib/HttpDownloadManager.cpp:
+
+ Fix bug 162681 in trunk (slow tile download).
+ The problem with the download implementation was that
+ there was only one QHttp object in HttpFetchFile, throught
+ which all requests were serialized.
+ Now we have got one QHttp object per active HttpJob, so
+ this bottleneck is gone now.
+ This fix was committed in trunk earlier this day.
+
2008-07-19 Inge Wallin <inge@lysator.liu.se>
Reduce warnings: Initialize in the right order.
--- branches/KDE/4.1/kdeedu/marble/src/lib/HttpDownloadManager.cpp #835028:835029
@@ -91,7 +91,7 @@
}
else
{
- delete job;
+ job->deleteLater();
}
}
@@ -109,7 +109,7 @@
}
else
{
- delete job;
+ job->deleteLater();
}
}
@@ -159,7 +159,7 @@
{
m_activatedJobList.removeAt ( pos );
qDebug() << "Removing: " << job->initiatorId();
- delete job;
+ job->deleteLater();
}
activateJobs();
--- branches/KDE/4.1/kdeedu/marble/src/lib/HttpFetchFile.cpp #835028:835029
@@ -24,62 +24,81 @@
m_destinationFileName ( destFileName ),
m_originalDestinationFileName ( destFileName ),
m_data(),
+ m_buffer( 0 ),
m_initiatorId ( id ),
m_status ( NoStatus ),
- m_priority ( NoPriority )
+ m_priority ( NoPriority ),
+ m_http( 0 )
{
- m_buffer = new QBuffer ( &m_data );
- m_buffer->open ( QIODevice::WriteOnly );
}
HttpJob::~HttpJob()
{
- m_buffer->close();
+
+ delete m_http;
delete m_buffer;
}
+void HttpJob::prepareExecution()
+{
+ // job can be executed more than once because of redirection
+ // perhaps better to make a new job than (FIXME)
+ if ( !m_http ) {
+ m_http = new QHttp;
+ m_buffer = new QBuffer ( &m_data );
+ m_buffer->open ( QIODevice::WriteOnly );
+ }
+}
+int HttpJob::execute()
+{
+ m_http->setHost( m_sourceUrl.host(),
+ m_sourceUrl.port() != -1 ? m_sourceUrl.port() : 80 );
+ if ( !m_sourceUrl.userName().isEmpty() )
+ m_http->setUser( m_sourceUrl.userName(), m_sourceUrl.password() );
+ // if Url has query item like in panoramio API requests source.path()
+ // chops it , this "if" gurantees its correct treatement
+ QString cleanupPath;
+ if ( m_sourceUrl.hasQuery() == true ) {
+ cleanupPath = QString( m_sourceUrl.toString( QUrl::RemoveAuthority | \
QUrl::RemoveScheme ) ); + }
+ else {
+ cleanupPath = QUrl::toPercentEncoding( m_sourceUrl.path(), "/", " -" );
+ }
+
+ qDebug() << m_sourceUrl.host() << "and path=" << cleanupPath;
+ QHttpRequestHeader header ( QLatin1String ( "GET" ), cleanupPath );
+ header.setValue ( "Connection", "Keep-Alive" );
+ header.setValue ( "User-Agent", "Marble TinyWebBrowser" );
+ header.setValue ( "Host", m_sourceUrl.host() );
+
+ const int httpGetId = m_http->request( header, 0, m_buffer );
+ return httpGetId;
+}
+
HttpFetchFile::HttpFetchFile ( StoragePolicy *policy, QObject *parent )
: QObject ( parent ),
m_storagePolicy ( policy )
{
- m_pHttp = new QHttp ( this );
-
- connect ( m_pHttp, SIGNAL ( requestFinished ( int, bool ) ),
- this, SLOT ( httpRequestFinished ( int, bool ) ) );
}
HttpFetchFile::~HttpFetchFile()
{
- m_pHttp->abort();
- delete m_pHttp;
}
void HttpFetchFile::executeJob ( HttpJob* job )
{
- const QUrl sourceUrl = job->sourceUrl();
+ // FIXME: this is a little bit ugly, but it resolves the following issues:
+ // 1. not all jobs in the queue should have QHttp allocated,
+ // but only the active ones
+ // 2. we have to connect before execution, even if it is asynchronously,
+ // I think.
+ job->prepareExecution();
- m_pHttp->setHost ( sourceUrl.host(), sourceUrl.port() != -1 ? sourceUrl.port() : \
80 );
- if ( !sourceUrl.userName().isEmpty() )
- m_pHttp->setUser ( sourceUrl.userName(), sourceUrl.password() );
-// if Url has query item like in panoramio API requests source.path() chops it , \
this "if" gurantees its correct treatement
- QString cleanupPath;
- if ( sourceUrl.hasQuery() == true )
- {
- cleanupPath= QString ( sourceUrl.toString ( QUrl::RemoveAuthority | \
QUrl::RemoveScheme ) );
- }
- else
- {
- cleanupPath = QUrl::toPercentEncoding ( sourceUrl.path(), "/", " -" );
- }
+ connect( job->m_http, SIGNAL( requestFinished( int, bool ) ),
+ this, SLOT( httpRequestFinished( int, bool ) ) );
- qDebug() <<sourceUrl.host() <<"and path="<<cleanupPath;
- QHttpRequestHeader header ( QLatin1String ( "GET" ), cleanupPath );
- header.setValue ( "Connection", "Keep-Alive" );
- header.setValue ( "User-Agent", "Marble TinyWebBrowser" );
- header.setValue ( "Host", sourceUrl.host() );
-
- int httpGetId = m_pHttp->request ( header, 0, job->buffer() );
+ int httpGetId = job->execute();
m_pJobMap.insert ( httpGetId, job );
emit statusMessage ( tr ( "Downloading data..." ) );
@@ -90,7 +109,9 @@
if ( !m_pJobMap.contains ( requestId ) )
return;
- QHttpResponseHeader responseHeader = m_pHttp->lastResponse();
+ HttpJob * job = m_pJobMap[ requestId ];
+
+ QHttpResponseHeader responseHeader = job->m_http->lastResponse();
// qDebug() << "responseHeader.statusCode():" << responseHeader.statusCode()
// << responseHeader.reasonPhrase();
@@ -100,8 +121,6 @@
// if ( responseHeader.isValid() == false )
// return;
- HttpJob* job = m_pJobMap[ requestId ];
-
if ( responseHeader.statusCode() == 301 )
{
QUrl newLocation ( responseHeader.value ( "Location" ) );
@@ -126,7 +145,7 @@
if ( error != 0 )
{
emit statusMessage ( tr ( "Download failed: %1." )
- .arg ( m_pHttp->errorString() ) );
+ .arg ( job->m_http->errorString() ) );
emit jobDone ( m_pJobMap[ requestId ], error );
m_pJobMap.remove ( requestId );
--- branches/KDE/4.1/kdeedu/marble/src/lib/HttpFetchFile.h #835028:835029
@@ -30,12 +30,22 @@
class QHttp;
class StoragePolicy;
-class HttpJob
+class HttpJob: public QObject
{
+ Q_OBJECT
+
public:
HttpJob( const QUrl & sourceUrl, const QString & destFileName, QString const id \
); ~HttpJob();
+ // allocates QHttp and QBuffer member, has to be done before
+ // execute() because of signal connections.
+ // see FIXME in .cpp
+ void prepareExecution();
+
+ // async, returns http get id
+ int execute();
+
QUrl sourceUrl() const;
void setSourceUrl( const QUrl & );
@@ -64,6 +74,8 @@
QString m_initiatorId;
Status m_status;
Priority m_priority;
+ public:
+ QHttp *m_http; // FIXME: cleans this up after 4.1
};
@@ -95,7 +107,6 @@
private:
Q_DISABLE_COPY( HttpFetchFile )
- QHttp *m_pHttp;
QMap<int, HttpJob*> m_pJobMap;
StoragePolicy *m_storagePolicy;
};
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic