On 06/28/2012 12:44 AM, Vishesh Handa wrote:


On Wed, Jun 27, 2012 at 1:22 AM, Sebastian Trüg <[email protected]
<mailto:[email protected]>> wrote:

    On 06/26/2012 10:42 AM, Vishesh Handa wrote:



        On Tue, Jun 26, 2012 at 1:17 PM, Sebastian Trüg <[email protected]
        <mailto:[email protected]>
        <mailto:[email protected] <mailto:[email protected]>>> wrote:

            On 06/25/2012 10:50 AM, Vishesh Handa wrote:

                Hey Sebastian

                The reviewboard has gone insane and it refuses to let me
        upload
                my patch
                - "The file
        'nepomuk/kioslaves/search/kio_____nepomuksearch.cpp'

                (r2311b7a)
                could not be found in the repository".

                The auto updates donot seem to work any more, so that really
                needs to be

         >

                fixed or thrown away. I would prefer throwing it away,
        but it
                does have
                a valid use-case, though the implementation is horrible.


            Why is the implementation horrible? I always thought it to
        be very
            clean. The concept is simple: the kio slave lists a folder which
            results in a KDirNotify signal which is used by the kded
        module to
            list the query in the query service. Since before that would

            have reused the same query folder there would have been no
            re-querying. The kded would simply subscribe to updates from the
            query service.


        I was referring to how the query service does the updates - It
        runs the
        queries every time any data has changed. The current implementation
        makes Nepomuk completely unusable. Mainly cause I have the
        telepathy-nepomuk-service running which updates the nepomuk repo any
        time the status of any of my contacts changes.

        Right now, the query will be run both in the kio-slave, and in the
        queryservice. Maybe we could abstract away the queryservice
        code, and
        make it run in the kioslave itself. That way the kded will
        listen to the
        kioslave for updates, and the queries will not have to be run twice.


    The kio slave does not run forever. So you cannot do updates that way.
    But remember that we had the idea to use the resource watcher to do
    some heuristics.


Hmm. So now what? Should I commit this patch?

I remember the plan of using some heuristics, I just don't think that
would help a lot. For simple queries like listing all your music files -
On my desktop that is over 20,000 files. Doing that query and processing
20000 files again, just cause one music file was added seems to be an
overkill.

I rather not have this feature of automatic updates, if it means such a
heavy performance penalty.

If you are only talking KIO then I suppose that would be OK. I would, however, like to keep the kded code around for when we improved the query updating.






                commit 1fba20495fddfc596591f6ea8535d5____fc6427c62b

                Author: Vishesh Handa<[email protected] <mailto:[email protected]>
        <mailto:[email protected] <mailto:[email protected]>>
        <mailto:[email protected] <mailto:[email protected]> <mailto:[email protected]
        <mailto:[email protected]>>>>


                Date:   Sat Jun 16 02:57:57 2012 +0530

                     Avoid using the query service to run queries in the
        kioslave

                     Avoid creating a new thread in the query service, and
                parsing the
                     results over dbus. Instead just run the query
        manually in
                the kioslave.

                     We can do this cause kioslaves run in a separate
        process
                and are allowed
                     to be blocking.

                diff --git a/nepomuk/kioslaves/common/____resourcestat.cpp
                b/nepomuk/kioslaves/common/____resourcestat.cpp
                index a259ba5..8f3088e 100644
                --- a/nepomuk/kioslaves/common/____resourcestat.cpp
                +++ b/nepomuk/kioslaves/common/____resourcestat.cpp

                @@ -349,6 +349,7 @@ KUrl Nepomuk2::redirectionUrl( const
                Nepomuk2::Resource&  res )


                  namespace {
                +
                      /**
                       * Check if the resource represents a local file
        with an
                existing nie:url property.
                       */
                diff --git
        a/nepomuk/kioslaves/search/____kio_nepomuksearch.cpp
                b/nepomuk/kioslaves/search/____kio_nepomuksearch.cpp
                index 2311b7a..3c82796 100644
                --- a/nepomuk/kioslaves/search/____kio_nepomuksearch.cpp
                +++ b/nepomuk/kioslaves/search/____kio_nepomuksearch.cpp
                @@ -182,7 +182,7 @@ void
        Nepomuk2::SearchProtocol::____listDir(

                const KUrl&  url )
                          else {
                              SearchFolder folder( url, this );
                              updateQueryUrlHistory( url );
                -            folder.waitForListing();
                +            folder.list();
                              listEntry( KIO::UDSEntry(), true );
                              finished();
                          }
                @@ -315,7 +315,7 @@ void
        Nepomuk2::SearchProtocol::____listRoot()

                      if ( query.isValid() ) {
                          // FIXME: Avoid this useless conversion to
        searchUrl
                and back
                          SearchFolder folder( query.toSearchUrl(), this );
                -        folder.waitForListing();
                +        folder.list();
                      }

                      listEntry( KIO::UDSEntry(), true );
                diff --git a/nepomuk/kioslaves/search/____searchfolder.cpp
                b/nepomuk/kioslaves/search/____searchfolder.cpp
                index 3cd098a..38e4b1d 100644
                --- a/nepomuk/kioslaves/search/____searchfolder.cpp
                +++ b/nepomuk/kioslaves/search/____searchfolder.cpp

                @@ -32,15 +32,19 @@
                  #include<Nepomuk2/Thing>
                  #include<Nepomuk2/Types/Class>
                  #include<Nepomuk2/Query/Query>
                -#include<Nepomuk2/Query/____QueryParser>
                +#include<Nepomuk2/Query/____Result>
                  #include<Nepomuk2/Query/____ResourceTypeTerm>
                -#include<Nepomuk2/Query/____QueryServiceClient>
                  #include<Nepomuk2/Vocabulary/____NFO>
                  #include<Nepomuk2/Vocabulary/____NIE>
                  #include<Nepomuk2/Vocabulary/____PIMO>

                +#include<Nepomuk2/____ResourceManager>

                +#include<Nepomuk2/Resource>
                +
                  #include<QtCore/QMutexLocker>
                  #include<QTextDocument>
                +#include<Soprano/____QueryResultIterator>

                +#include<Soprano/Model>

                  #include<KUrl>
                  #include<KDebug>
                @@ -51,6 +55,8 @@
                  #include<KConfig>
                  #include<KConfigGroup>

                +using namespace Nepomuk2::Vocabulary;
                +using namespace Soprano::Vocabulary;

                  Nepomuk2::SearchFolder::____SearchFolder( const KUrl&
          url,

                KIO::SlaveBase* slave )
                      : QObject( 0 ),
                @@ -60,23 +66,9 @@
        Nepomuk2::SearchFolder::____SearchFolder( const

                KUrl&  url, KIO::SlaveBase* slave )
                      // parse URL (this may fail in which case we fall
        back to
                pure SPARQL below)
                      Query::parseQueryUrl( url, m_query, m_sparqlQuery );

                -    m_client = new
        Nepomuk2::Query::____QueryServiceClient();

                -
                -    connect( m_client, SIGNAL( newEntries( const
                QList<Nepomuk2::Query::Result>____&  ) ),

                -             this, SLOT( slotNewEntries( const
                QList<Nepomuk2::Query::Result>____&  ) ) );

                -    connect( m_client, SIGNAL( resultCount(int) ),
                -             this, SLOT( slotResultCount(int) ) );
                -    connect( m_client, SIGNAL( finishedListing() ),
                -             this, SLOT( slotFinishedListing() ) );
                -    connect( m_client, SIGNAL( error(QString) ),
                -             this, SLOT( slotFinishedListing() ) );
                -    connect( m_client, SIGNAL( finishedListing() ),
                -             m_client, SLOT( deleteLater() ) );
                -
                -    if ( m_query.isValid() )
                -        m_client->query( m_query );
                -    else
                -        m_client->sparqlQuery( m_sparqlQuery );
                +    if ( m_query.isValid() ) {
                +        m_sparqlQuery = m_query.toSparqlQuery();
                +    }
                  }


                @@ -84,36 +76,20 @@
        Nepomuk2::SearchFolder::~____SearchFolder()
                  {
                  }

                -void Nepomuk2::SearchFolder::____waitForListing()

                -{
                -    m_eventLoop.exec();
                -}
                -
                -void Nepomuk2::SearchFolder::____slotNewEntries( const
                QList<Nepomuk2::Query::Result>____&  results )

                +void Nepomuk2::SearchFolder::list()
                  {
                -    KIO::UDSEntryList entryList;
                -    foreach(const Query::Result&  result, results ) {
                +    //FIXME: Do the result count as well?
                +    Soprano::Model* model =
                ResourceManager::instance()->____mainModel();

                +    Soprano::QueryResultIterator it = model->executeQuery(
                m_sparqlQuery, Soprano::Query::____QueryLanguageSparql );

                +    while( it.next() ) {
                +        Query::Result result = extractResult( it );
                          KIO::UDSEntry uds = statResult( result );
                          if ( uds.count() ) {
                -            //kDebug()<< "listing" <<
                  result.resource().resourceUri(____);
                              m_slave->listEntry(uds, false);
                          }
                      }
                  }

                -
                -void Nepomuk2::SearchFolder::____slotResultCount( int
        count )

                -{
                -    m_slave->totalSize( count );
                -}
                -
                -
                -void Nepomuk2::SearchFolder::____slotFinishedListing()

                -{
                -    m_eventLoop.exit();
                -}
                -
                -
                  namespace {
                      bool statFile( const KUrl&  url, const KUrl&  fileUrl,
                KIO::UDSEntry&  uds )
                      {
                @@ -146,7 +122,7 @@ KIO::UDSEntry
                Nepomuk2::SearchFolder::____statResult( const
        Query::Result&  result )

                  {
                      Resource res( result.resource() );
                      const KUrl uri( res.resourceUri() );
                -    KUrl nieUrl(
                result[Nepomuk2::Vocabulary::____NIE::url()].uri() );

                +    KUrl nieUrl( result[NIE::url()].uri() );

                      // the additional bindings that we only have on
        unix systems
                      // Either all are bound or none of them.
                @@ -263,3 +239,36 @@ KIO::UDSEntry
                Nepomuk2::SearchFolder::____statResult( const
        Query::Result&  result )


                      return uds;
                  }
                +
                +// copied from the QueryService
                +Nepomuk2::Query::Result
                Nepomuk2::SearchFolder::____extractResult(const

                Soprano::QueryResultIterator&  it) const
                +{
                +    Query::Result result( Resource::fromResourceUri(
                it[0].uri() ) );
                +    const Query::RequestPropertyMap map =
                m_query.requestPropertyMap();
                +    for( Query::RequestPropertyMap::____const_iterator
        rit =

                map.begin(); rit != map.constEnd(); rit++ ) {
                +        result.addRequestProperty( rit.value(), it.binding(
                rit.key() ) );
                +    }
                +
                +    // make sure we do not store values twice
                +    QStringList names = it.bindingNames();
                +    names.removeAll( QLatin1String("r"  ) );
                +
                +    static const char* s_scoreVarName ="_n_f_t_m_s_";
                +    static const char* s_excerptVarName ="_n_f_t_m_ex_";
                +
                +    Soprano::BindingSet set;
                +    int score = 0;
                +    Q_FOREACH( const QString&  var, names ) {
                +        if ( var == QLatin1String( s_scoreVarName ) )
                +            score = it[var].literal().toInt();
                +        else if ( var == QLatin1String(
        s_excerptVarName ) )
                +            result.setExcerpt( it[var].toString() );
                +        else
                +            set.insert( var, it[var] );
                +    }
                +
                +    result.setAdditionalBindings( set );
                +    result.setScore( ( double )score );
                +
                +    return result;
                +}
                \ No newline at end of file
                diff --git a/nepomuk/kioslaves/search/____searchfolder.h
                b/nepomuk/kioslaves/search/____searchfolder.h
                index 794baaa..b475402 100644
                --- a/nepomuk/kioslaves/search/____searchfolder.h
                +++ b/nepomuk/kioslaves/search/____searchfolder.h

                @@ -36,6 +36,9 @@
                  #include<Nepomuk2/Resource>
                  #include<KUrl>

                +namespace Soprano {
                +    class QueryResultIterator;
                +}

                  namespace Nepomuk2 {
                      namespace Query {
                @@ -76,17 +79,7 @@ namespace Nepomuk2 {
                          /**
                           * List the results directly on the parent slave.
                           */
                -        void waitForListing();
                -
                -    private Q_SLOTS:
                -        /// connected to the QueryServiceClient in the
        search
                thread
                -        void slotNewEntries( const
                QList<Nepomuk2::Query::Result>____&  );

                -
                -        /// connected to the QueryServiceClient in the
        search
                thread
                -        void slotResultCount( int );
                -
                -        /// connected to the QueryServiceClient in the
        search
                thread
                -        void slotFinishedListing();
                +        void list();

                      private:
                          /**
                @@ -94,6 +87,8 @@ namespace Nepomuk2 {
                           */
                          KIO::UDSEntry statResult( const Query::Result&
          result );

                +        Query::Result extractResult( const
                Soprano::QueryResultIterator&  it ) const;
                +
                          // folder properties
                          KUrl m_url;




                --
                Vishesh Handa




        --
        Vishesh Handa




--
Vishesh Handa

_______________________________________________
Nepomuk mailing list
[email protected]
https://mail.kde.org/mailman/listinfo/nepomuk

Reply via email to