[prev in list] [next in list] [prev in thread] [next in thread]
List: kde-commits
Subject: Re: [Amarok] b18a028: Use shared memory for collection scanner to
From: Maximilian Kossick <maximilian.kossick () googlemail ! com>
Date: 2010-11-06 9:38:55
Message-ID: AANLkTinDwkfZGtGrMM=QW1CtAUkgOnOw24Fx2d3rVjs4 () mail ! gmail ! com
[Download RAW message or body]
One of the reasons for collection scanner as a separate executable was
to prevent a crash in collectionscanner from bringing down Amarok as
well. As both processes only communicated with each other via files or
stdout/stdin, this was relatively safe. Is it ensured that a
corrpution of the shared memory data does not crash both processes?
On Sat, Nov 6, 2010 at 4:05 AM, Ralf Engels <ralf-engels@gmx.de> wrote:
> commit b18a028c66b4aff2df1df9cf63fe1eb712a9b8bd
> branch master
> Author: Ralf Engels <ralf-engels@gmx.de>
> Date: Fri Nov 5 19:14:32 2010 +0100
>
> Use shared memory for collection scanner to remember last scanning position \
> increasing the scanning speed. Fix problem with track and stream resume
>
> diff --git a/src/core-impl/collections/db/ScanManager.cpp \
> b/src/core-impl/collections/db/ScanManager.cpp index b02a80d..c145e08 100644
> --- a/src/core-impl/collections/db/ScanManager.cpp
> +++ b/src/core-impl/collections/db/ScanManager.cpp
> @@ -41,6 +41,7 @@
> #include <QListIterator>
> #include <QStringList>
> #include <QTextCodec>
> +#include <QSharedMemory>
>
> #include <KMessageBox>
> #include <KStandardDirs>
> @@ -53,12 +54,14 @@ static const int MAX_RESTARTS = 80;
> static const int MAX_FAILURE_PERCENTAGE = 5;
> static const int WATCH_INTERVAL = 60 * 1000; // = 60 seconds
>
> +static const int SHARED_MEMORY_SIZE = 1024 * 1024; // 1 MB shared memory
>
> ScanManager::ScanManager( Collections::DatabaseCollection *collection, QObject \
> *parent ) : QObject( parent )
> , m_collection( static_cast<Collections::SqlCollection*>( collection ) )
> , m_scanner( 0 )
> , m_parser( 0 )
> + , m_scannerStateMemory( 0 )
> , m_restartCount( 0 )
> , m_blockCount( 0 )
> , m_fullScanRequested( false )
> @@ -218,6 +221,20 @@ ScanManager::startScannerProcess( bool restart )
> Q_ASSERT( m_parser );
> Q_ASSERT( !m_scanner );
>
> + // -- create the shared memory
> + if( !m_scannerStateMemory && !restart )
> + {
> + m_sharedMemoryKey = "AmarokScannerMemory"+QDateTime::current().toString();
> + m_scannerStateMemory = new QSharedMemory( m_sharedMemoryKey, this );
> + if( !m_scannerStateMemory.create( SHARED_MEMORY_SIZE ) )
> + {
> + warning() << "Unable to create shared memory for collection scanner";
> + delete m_scannerStateMemory;
> + m_scannerStateMemory = 0;
> + }
> + }
> +
> + // -- create the scanner process
> m_scanner = new AmarokProcess( this );
> *m_scanner << scannerPath() << "--idlepriority";
> if( !m_fullScanRequested )
> @@ -232,6 +249,9 @@ ScanManager::startScannerProcess( bool restart )
> if( restart )
> *m_scanner << "-s";
>
> + if( m_scannerStateMemory )
> + *m_scanner << "--sharedmemory" << m_sharedMemoryKey;
> +
> *m_scanner << "--batch" << m_batchfilePath;
>
> m_scanner->setOutputChannelMode( KProcess::OnlyStdoutChannel );
> @@ -300,15 +320,14 @@ ScanManager::slotFinished(int exitCode, QProcess::ExitStatus \
> exitStatus)
> {
> QMutexLocker locker( &m_mutex );
> - m_scanner->terminate();
> - m_scanner->deleteLater();
> - m_scanner = 0;
> - m_restartCount = 0;
> -
> - m_fullScan = false;
> - m_scanDirs.clear();
> + if( m_scanner )
> + {
> + stopScanner();
>
> - QFile::remove( m_batchfilePath );
> + m_restartCount = 0;
> + m_fullScan = false;
> + m_scanDirs.clear();
> + }
> }
> }
>
> @@ -337,15 +356,10 @@ ScanManager::abort( const QString &reason )
> QMutexLocker locker( &m_mutex );
> if( m_scanner )
> {
> - disconnect( m_scanner, 0, this, 0 );
> - m_scanner->terminate();
> - m_scanner->deleteLater();
> - m_scanner = 0;
> + stopScanner();
>
> m_fullScan = false;
> m_scanDirs.clear();
> -
> - QFile::remove( m_batchfilePath );
> }
> }
>
> @@ -426,6 +440,23 @@ ScanManager::handleRestart()
> void
> ScanManager::stopParser()
> {
> + // stop the scanner
> + disconnect( m_scanner, 0, this, 0 );
> + m_scanner->terminate();
> + m_scanner->deleteLater();
> + m_scanner = 0;
> +
> + // free it's shared memory.
> + delete m_scannerStateMemory;
> + m_scannerStateMemory = 0;
> +
> + // remove the batch file
> + QFile::remove( m_batchfilePath );
> +}
> +
> +void
> +ScanManager::stopParser()
> +{
> QMutexLocker locker( &m_mutex );
> if( m_parser )
> {
> diff --git a/src/core-impl/collections/db/ScanManager.h \
> b/src/core-impl/collections/db/ScanManager.h index 9fcdafd..4e20cbb 100644
> --- a/src/core-impl/collections/db/ScanManager.h
> +++ b/src/core-impl/collections/db/ScanManager.h
> @@ -35,6 +35,7 @@
> #include <threadweaver/Job.h>
>
> class XmlParseJob;
> +class QSharedMemory;
>
> class AMAROK_DATABASECOLLECTION_EXPORT_TESTS ScanManager : public QObject
> {
> @@ -110,12 +111,15 @@ class AMAROK_DATABASECOLLECTION_EXPORT_TESTS ScanManager : \
> public QObject
> void handleRestart();
>
> + void stopScanner();
> void stopParser();
>
> private:
> Collections::DatabaseCollection *m_collection;
>
> AmarokProcess *m_scanner;
> + QSharedMemory *m_scannerStateMemory; // a persistent storage of the \
> current scanner state in case it needs to be restarted. + QString \
> m_sharedMemoryKey; XmlParseJob *m_parser;
>
> int m_restartCount;
> diff --git a/utilities/collectionscanner/CollectionScanner.cpp \
> b/utilities/collectionscanner/CollectionScanner.cpp index 7a4b588..e188272 100644
> --- a/utilities/collectionscanner/CollectionScanner.cpp
> +++ b/utilities/collectionscanner/CollectionScanner.cpp
> @@ -36,10 +36,13 @@
> #include <QStringList>
> #include <QDir>
> #include <QFile>
> -#include <QSettings>
> #include <QDateTime>
> #include <QXmlStreamReader>
> #include <QXmlStreamWriter>
> +#include <QSharedMemory>
> +#include <QByteArray>
> +#include <QTextStream>
> +#include <QDebug>
>
> #include <audiblefiletyperesolver.h>
> #include <realmediafiletyperesolver.h>
> @@ -54,6 +57,157 @@ main( int argc, char *argv[] )
> return scanner.exec();
> }
>
> +
> +// ------------ the scanning state -----------
> +
> +CollectionScanner::ScanningState::ScanningState()
> + : m_sharedMemory(0)
> +{
> +}
> +
> +CollectionScanner::ScanningState::~ScanningState()
> +{
> + delete m_sharedMemory;
> +}
> +
> +void
> +CollectionScanner::ScanningState::setKey( const QString &key )
> +{
> + delete m_sharedMemory;
> + m_sharedMemory = new QSharedMemory( key );
> + if( m_sharedMemory->attach() )
> + readFull();
> +}
> +
> +bool
> +CollectionScanner::ScanningState::isValid() const
> +{
> + return m_sharedMemory && m_sharedMemory->isAttached();
> +}
> +
> +QString
> +CollectionScanner::ScanningState::lastDirectory() const
> +{ return m_lastDirectory; }
> +
> +void
> +CollectionScanner::ScanningState::setLastDirectory( const QString &dir )
> +{
> + if( dir == m_lastDirectory )
> + return;
> +
> + m_lastDirectory = dir;
> + writeFull();
> +}
> +
> +QStringList
> +CollectionScanner::ScanningState::directories() const
> +{ return m_directories; }
> +
> +void
> +CollectionScanner::ScanningState::setDirectories( const QStringList &directories )
> +{
> + if( directories == m_directories )
> + return;
> +
> + m_directories = directories;
> + writeFull();
> +}
> +
> +QStringList
> +CollectionScanner::ScanningState::badFiles() const
> +{ return m_badFiles; }
> +
> +void
> +CollectionScanner::ScanningState::setBadFiles( const QStringList &badFiles )
> +{
> + if( badFiles == m_badFiles )
> + return;
> +
> + m_badFiles = badFiles;
> + writeFull();
> +}
> +
> +QString
> +CollectionScanner::ScanningState::lastFile() const
> +{ return m_lastFile; }
> +
> +void
> +CollectionScanner::ScanningState::setLastFile( const QString &file )
> +{
> + if( file == m_lastFile )
> + return;
> +
> + m_sharedMemory->lock();
> + QByteArray data = QByteArray::fromRawData((char*)m_sharedMemory->data(), \
> m_sharedMemory->size()); + QTextStream out( &data );
> + out.seek( m_lastFilePos );
> + out << m_lastFile;
> + m_sharedMemory->unlock();
> +}
> +
> +void
> +CollectionScanner::ScanningState::readFull()
> +{
> + if( !isValid() )
> + return;
> +
> + m_sharedMemory->lock();
> + QByteArray data = QByteArray::fromRawData((char*)m_sharedMemory->data(), \
> m_sharedMemory->size()); + QTextStream in(&data, QIODevice::ReadOnly);
> +
> + int count;
> +
> + in >> m_lastDirectory;
> +
> + m_directories.clear();
> + in >> count;
> + for( int i=0; i<count; i++ )
> + {
> + QString s;
> + in >> s;
> + m_directories.append( s );
> + }
> +
> + m_badFiles.clear();
> + in >> count;
> + for( int i=0; i<count; i++ )
> + {
> + QString s;
> + in >> s;
> + m_badFiles.append( s );
> + }
> +
> + m_lastFilePos = in.pos();
> + in >> m_lastFile;
> +
> + m_sharedMemory->unlock();
> +}
> +
> +void
> +CollectionScanner::ScanningState::writeFull()
> +{
> + if( !isValid() )
> + return;
> +
> + m_sharedMemory->lock();
> + QByteArray data = QByteArray::fromRawData((char*)m_sharedMemory->data(), \
> m_sharedMemory->size()); + QTextStream out( &data );
> + out << m_lastDirectory;
> + out << m_directories.count();
> + foreach( const QString &s, m_directories )
> + out << s;
> + out << m_badFiles.count();
> + foreach( const QString &s, m_badFiles )
> + out << s;
> + m_lastFilePos = out.pos();
> + out << m_lastFile;
> + qWarning("shared memory at %d of %d", out.pos(), m_sharedMemory->size());
> + m_sharedMemory->unlock();
> +}
> +
> +
> +// ------------ the scanner -----------
> +
> CollectionScanner::Scanner::Scanner( int &argc, char **argv )
> > QCoreApplication( argc, argv )
> , m_charset( false )
> @@ -78,19 +232,6 @@ CollectionScanner::Scanner::Scanner( int &argc, char **argv )
> TagLib::FileRef::addFileTypeResolver(new ASFFileTypeResolver);
> TagLib::FileRef::addFileTypeResolver(new MP4FileTypeResolver);
> TagLib::FileRef::addFileTypeResolver(new WAVFileTypeResolver);
> -
> - QString env = qgetenv( "KDEHOME" );
> - if( env.isEmpty() || !QDir(env).isReadable() )
> - env = QString( "%1/.kde" ).arg( QDir::homePath() );
> - QString path = env + QLatin1String("/share/apps");
> -
> - // give two different settings.
> - // prevent the possibility that an incremental and a non-incremental scan \
> clash
> - QSettings::setPath( QSettings::NativeFormat, QSettings::UserScope, path );
> - if( m_incremental )
> - m_settings = new QSettings( "amarok", "CollectionScannerIncremental", this \
> );
> - else
> - m_settings = new QSettings( "amarok", "CollectionScanner", this );
> }
>
>
> @@ -144,11 +285,11 @@ CollectionScanner::Scanner::doJob() //SLOT
> // --- determine the directories we have to scan
> QStringList entries;
>
> - // -- when restarting read them from the settings
> - if( m_restart )
> + // -- when restarting read them from the shared memory
> + if( m_restart && m_scanningState.isValid() )
> {
> - QString lastEntry = m_settings->value( "lastDirectory" ).toString();
> - entries = m_settings->value( "directories" ).toStringList();
> + QString lastEntry = m_scanningState.lastDirectory();
> + entries = m_scanningState.directories();
>
> // remove the entries we already scanned
> while( entries.front() != lastEntry && !entries.empty() )
> @@ -180,8 +321,8 @@ CollectionScanner::Scanner::doJob() //SLOT
> }
>
> entries = entriesSet.toList();
> - m_settings->setValue( "lastDirectory", QString() );
> - m_settings->setValue( "directories", entries );
> + m_scanningState.setLastDirectory( QString() );
> + m_scanningState.setDirectories( entries );
> }
>
> if( !m_restart )
> @@ -194,7 +335,7 @@ CollectionScanner::Scanner::doJob() //SLOT
> // --- now do the scanning
> foreach( QString path, entries )
> {
> - CollectionScanner::Directory dir( path, m_settings,
> + CollectionScanner::Directory dir( path, &m_scanningState,
> m_incremental && !isModified( path ) );
>
> xmlWriter.writeStartElement( "directory" );
> @@ -295,6 +436,14 @@ CollectionScanner::Scanner::readArgs()
> missingArg = true;
> argnum++;
> }
> + else if( myarg == "sharedmemory" )
> + {
> + if( argslist.count() > argnum + 1 )
> + m_scanningState.setKey( argslist.at( argnum + 1 ) );
> + else
> + missingArg = true;
> + argnum++;
> + }
> else if( myarg == "version" )
> displayVersion();
> else if( myarg == "incremental" )
> @@ -392,6 +541,7 @@ CollectionScanner::Scanner::displayHelp( const QString &error )
> "-i, --incremental : Incremental scan (modified folders only)\n"
> "-s, --restart : After a crash, restart the scanner in its last \
> position\n" " --idlepriority : Run at idle priority\n"
> + " --sharedmemory <key> : A shared memory segment to be used for \
> restarting a scan\n" " --newer <path> : Only scan directories if \
> modification time is new than <path>\n" " Only useful in \
> incremental scan mode\n" " --batch <path> : Add the directories from the \
> batch xml file\n"
> diff --git a/utilities/collectionscanner/CollectionScanner.h \
> b/utilities/collectionscanner/CollectionScanner.h index 4a21248..4968c98 100644
> --- a/utilities/collectionscanner/CollectionScanner.h
> +++ b/utilities/collectionscanner/CollectionScanner.h
> @@ -33,11 +33,49 @@
> #include <QSet>
> #include <QStringList>
>
> -class QSettings;
> +class QSharedMemory;
>
> namespace CollectionScanner
> {
>
> +/** A class used to store the current scanning state in a shared memory segment.
> + Storing the state of the scanner shouldn't cause a file access.
> + We are using a shared memory that the amarok process holds open until the \
> scanning + is finished to store the state.
> + */
> +class ScanningState
> +{
> + public:
> + ScanningState();
> + ~ScanningState();
> +
> + void setKey( const QString &key );
> + bool isValid() const;
> +
> + QString lastDirectory() const;
> + void setLastDirectory( const QString &dir );
> + QStringList directories() const;
> + void setDirectories( const QStringList &directories );
> +
> + QStringList badFiles() const;
> + void setBadFiles( const QStringList &badFiles );
> + QString lastFile() const;
> + void setLastFile( const QString &file );
> +
> + private:
> + void readFull();
> + void writeFull();
> +
> + QSharedMemory *m_sharedMemory;
> +
> + QString m_lastDirectory;
> + QStringList m_directories;
> + QStringList m_badFiles;
> + QString m_lastFile;
> + qint64 m_lastFilePos;
> +};
> +
> +
> /**
> * @class Scanner
> * @short Scans directories and builds the Collection
> @@ -92,8 +130,7 @@ private:
> bool m_idlePriority;
>
> QString m_mtimeFile;
> -
> - QSettings* m_settings;
> + ScanningState m_scanningState;
>
>
> // Disable copy constructor and assignment
> diff --git a/utilities/collectionscanner/Directory.cpp \
> b/utilities/collectionscanner/Directory.cpp index 76ceb99..7acd8ed 100644
> --- a/utilities/collectionscanner/Directory.cpp
> +++ b/utilities/collectionscanner/Directory.cpp
> @@ -37,7 +37,9 @@
>
> #ifdef UTILITIES_BUILD
>
> -CollectionScanner::Directory::Directory( const QString &path, QSettings *settings, \
> bool skip ) +CollectionScanner::Directory::Directory( const QString &path,
> + CollectionScanner::ScanningState *state,
> + bool skip )
> > m_ignored( false )
> {
> m_path = path;
> @@ -55,26 +57,26 @@ CollectionScanner::Directory::Directory( const QString &path, \
> QSettings *setting return;
> }
>
> -
> - QStringList validImages; validImages << "jpg" << "png" << "gif" << \
> "jpeg" << "bmp";
> - QStringList validPlaylists; validPlaylists << "m3u" << "pls" << "xspf";
> + QStringList validImages;
> + validImages << "jpg" << "png" << "gif" << "jpeg" << "bmp" << "svg" << "xpm";
> + QStringList validPlaylists;
> + validPlaylists << "m3u" << "pls" << "xspf";
>
> // --- check if we were restarted and failed at a file
> QStringList badFiles;
>
> - QString lastEntry = settings->value( "lastDirectory" ).toString();
> - if( lastEntry == path )
> + if( state->lastDirectory() == path )
> {
> - badFiles << settings->value( "lastFile" ).toString();
> - badFiles << settings->value( "badFiles" ).toStringList();
> + badFiles << state->badFiles();
> + badFiles << state->lastFile();
>
> - settings->setValue( "badFiles", badFiles );
> + state->setBadFiles( badFiles );
> }
> else
> {
> - settings->setValue( "lastDirectory", path );
> - settings->setValue( "lastFile", QString() );
> - settings->setValue( "badFiles", QStringList() );
> + state->setLastDirectory( path );
> + state->setLastFile( QString() );
> + state->setBadFiles( badFiles );
> }
>
> QStringList covers;
> @@ -92,10 +94,6 @@ CollectionScanner::Directory::Directory( const QString &path, \
> QSettings *setting if( badFiles.contains( f.absoluteFilePath() ) )
> continue;
>
> - // remember the last file before it get's dangerous. Before starting \
> taglib
> - settings->setValue( "lastFile", f.absoluteFilePath() );
> - settings->sync();
> -
> const QString suffix = fi.suffix().toLower();
> const QString filePath = f.absoluteFilePath();
>
> @@ -103,8 +101,8 @@ CollectionScanner::Directory::Directory( const QString &path, \
> QSettings *setting if( validImages.contains( suffix ) )
> {
> covers += filePath;
> -
> }
> +
> // -- playlist ?
> else if( validPlaylists.contains( suffix ) )
> {
> @@ -114,6 +112,9 @@ CollectionScanner::Directory::Directory( const QString &path, \
> QSettings *setting // -- audio track ?
> else
> {
> + // remember the last file before it get's dangerous. Before starting \
> taglib + state->setLastFile( f.absoluteFilePath() );
> +
> CollectionScanner::Track newTrack( filePath );
> if( newTrack.isValid() )
> {
> diff --git a/utilities/collectionscanner/Directory.h \
> b/utilities/collectionscanner/Directory.h index 8ba28f7..091390f 100644
> --- a/utilities/collectionscanner/Directory.h
> +++ b/utilities/collectionscanner/Directory.h
> @@ -34,16 +34,17 @@ class QXmlStreamWriter;
> namespace CollectionScanner
> {
>
> +class ScanningState;
> +
> /**
> * @class Directory
> * @short Represents a scanned directory and it's contents
> */
> -
> class Directory
> {
> public:
> #ifdef UTILITIES_BUILD
> - Directory( const QString &path, QSettings *settings, bool skip = false );
> + Directory( const QString &path, ScanningState *state, bool skip );
> #endif // UTILITIES_BUILD
>
> /** Reads a directory from an xml stream.
> diff --git a/utilities/collectionscanner/Track.cpp \
> b/utilities/collectionscanner/Track.cpp index 9291662..d0458d7 100644
> --- a/utilities/collectionscanner/Track.cpp
> +++ b/utilities/collectionscanner/Track.cpp
> @@ -129,7 +129,7 @@ CollectionScanner::Track::Track( const QString &path)
> if( !fileref.isNull() )
> {
> tag = fileref.tag();
> - if ( tag )
> + if( tag )
> {
> m_title = strip( tag->title() );
> m_artist = strip( tag->artist() );
> @@ -404,7 +404,7 @@ CollectionScanner::Track::Track( const QString &path)
>
> m_filesize = QFile( path ).size();
>
> - if( m_uniqueid.isEmpty() )
> + if( m_valid && m_uniqueid.isEmpty() )
> {
> AFTUtility aftutil;
> m_uniqueid = "amarok-sqltrackuid://" + aftutil.readUniqueId( path );
>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic