Patch/Thoughts - Konqueror's history loading..

Maks Orlovich kde-optimize@mail.kde.org
Sat, 11 Jan 2003 12:21:54 -0500


--Boundary-00=_yKFI+kOQJ/Z5DAs
Content-Type: text/plain;
  charset="iso-8859-1"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline

On Friday 10 January 2003 07:40 am, Carsten Pfeiffer wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
>
> On Friday 10 January 2003 05:30, Maks Orlovich wrote:
>
> Hiya,
>
> thanks for doing this, Maks, I put this onto my TODO when I saw David's
> commit, but didn't get to do it yet  The patch looks good to me.
>
> When breaking compatibility anyway, we can also remove the two dummy values
> (not used anymore, used to be maxAge and maxCount) from the history
> saving/loading. Not that this will affect startup time much tho ;)

Like in the attached (warning: don't try this with history files generated by 
the previous version of the patch ;-) )?

By the way, I am having weird problems with Konqueror not calling saveHistory 
sometimes - i.e. if I apply a patch, go to a few websites, exit konqueror, I 
don't see file getting updated. But if I manually force file format update 
once it seems to work fine from then on.. Thoughts on what I may me missing?

>
> > Thus, I am not sure of whether to commit what I have right now, or
> > whether there can be some ways to speed up other parts. The 75 ms splits
> > up roughly as following:
> >
> > Creating entries, deserializing them    - 26ms
>
> Maybe using a KZoneAllocator for all those items helps a bit.

I don't remember the split, but IIRC the allocation of the entries themselves 
is actually kind of cheap, compared to the reads. Now, if QString where to 
use some sort of a zone allocator for at least the QStringData nodes, well, 
that would get interesting. 

> To David's remark:
> We could backup the old file for people who would like to downgrade. We'd
> need some kind of downgrade mechanism in KDE to restore it tho. E.g. on
> startup, the previously run KDE version is compared against the current
> version and some converters are run according to that information. But
> that's probably OT here.

Well, but the previous version wouldn't have that mechanism, no?

>
> Cheers
> Carsten Pfeiffer
> -----BEGIN PGP SIGNATURE-----
>
> iQEVAwUBPh6/JaWgYMJuwmZtAQF5xAgApkpI4mm7bLhUTls3iVMivxx54OpfNV+4
> 7cEzGIsbaiiB/j7zmqGDjIm9bMigq4LWKrjEFHvO2eMu0KQ0hqhr18+j2wgUTLZj
> NNDUv7TsesOcyavdypBl9o2jcDz1DAyHica2YpRcCfaLeRjcc9ZEv6bTQtOpMIZj
> Ct34fM9O3VfrclAF/F2aOtRXTw5yjwefy4H+ch5bGut01ELx1hilmbJd3EuEau+W
> Tk2nLiRwDYckvevHjLvpLkx/+9dc8GeT4uR4AeFuaF70ol084u3NDkS+994Gs158
> VWUQ+8C/CBvXwZ/zsirYxMK0KuD6LHsjD/ihWZ/HHQAfeXS+7escVA==
> =a+ip
> -----END PGP SIGNATURE-----
>
> _______________________________________________
> Kde-optimize mailing list
> Kde-optimize@mail.kde.org
> http://mail.kde.org/mailman/listinfo/kde-optimize

--Boundary-00=_yKFI+kOQJ/Z5DAs
Content-Type: text/x-diff;
  charset="iso-8859-1";
  name="opt_konqhist2.diff"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment; filename="opt_konqhist2.diff"

Index: konq_historymgr.cc
===================================================================
RCS file: /home/kde/kdebase/libkonq/konq_historymgr.cc,v
retrieving revision 1.32
diff -u -3 -p -r1.32 konq_historymgr.cc
--- konq_historymgr.cc	11 Aug 2002 20:30:07 -0000	1.32
+++ konq_historymgr.cc	11 Jan 2003 17:13:26 -0000
@@ -30,7 +30,7 @@
 
 #include <zlib.h>
 
-const Q_UINT32 KonqHistoryManager::s_historyVersion = 2;
+const Q_UINT32 KonqHistoryManager::s_historyVersion = 3;
 
 KonqHistoryManager::KonqHistoryManager( QObject *parent, const char *name )
     : KParts::HistoryProvider( parent, name ),
@@ -74,6 +74,7 @@ KonqHistoryManager::~KonqHistoryManager(
 // loads the entire history
 bool KonqHistoryManager::loadHistory()
 {
+    KonqHistoryEntry::loadStringURL = false;
     clearPending();
     m_history.clear();
     m_pCompletion->clear();
@@ -104,19 +105,29 @@ bool KonqHistoryManager::loadHistory()
         bool crcChecked = false;
         bool crcOk = false;
 
-        if ( version == 2 ) {
+        if ( version == 2 || version == 3) {
             Q_UINT32 crc;
-
             crcChecked = true;
-
             fileStream >> crc >> data;
-
             crcOk = crc32( 0, reinterpret_cast<unsigned char *>( data.data() ), data.size() ) == crc;
-
             stream = &crcStream; // pick up the right stream
         }
-        else if ( version == 1 ) // fake, as we still support v1
-            version = 2;
+
+	if ( version != 3 )
+	{
+	    //Turn on backwards compatibility mode..
+	    KonqHistoryEntry::loadStringURL = true;
+	    // it doesn't make sense to save to save maxAge and maxCount  in the
+	    // binary file, this would make backups impossible (they would clear
+	    // themselves on startup, because all entries expire).
+	    // [But V1 and V2 formats did it, so we do a dummy read]
+	    Q_UINT32 dummy;
+	    *stream >> dummy;
+	    *stream >> dummy;
+	    
+	    //OK.
+	    version = 3;
+	}
 
         if ( s_historyVersion != version || ( crcChecked && !crcOk ) ) {
 	    kdWarning() << "The history version doesn't match, aborting loading" << endl;
@@ -125,23 +136,16 @@ bool KonqHistoryManager::loadHistory()
 	    return false;
 	}
 
-	// it doesn't make sense to save to save maxAge and maxCount  in the
-	// binary file, this would make backups impossible (they would clear
-	// themselves on startup, because all entries expire).
-	Q_UINT32 dummy;
-        *stream >> dummy;
-        *stream >> dummy;
 
         while ( !stream->atEnd() ) {
 	    KonqHistoryEntry *entry = new KonqHistoryEntry;
 	    Q_CHECK_PTR( entry );
             *stream >> *entry;
-
 	    // kdDebug(1203) << "## loaded entry: " << entry->url << ",  Title: " << entry->title << endl;
 	    m_history.append( entry );
+	    QString urlString2 = entry->url.prettyURL();    
 
-	    // insert the completion item weighted
- 	    m_pCompletion->addItem( entry->url.prettyURL(),
+	    m_pCompletion->addItem( urlString2,
 				    entry->numberOfTimesVisited );
  	    m_pCompletion->addItem( entry->typedURL,
 				    entry->numberOfTimesVisited );
@@ -151,7 +155,7 @@ bool KonqHistoryManager::loadHistory()
 	    KParts::HistoryProvider::insert( urlString );
             // DF: also insert the "pretty" version if different
             // This helps getting 'visited' links on websites which don't use fully-escaped urls.
-            QString urlString2 = entry->url.prettyURL();
+        
             if ( urlString != urlString2 )
                 KParts::HistoryProvider::insert( urlString2 );
 	}
@@ -188,9 +192,6 @@ bool KonqHistoryManager::saveHistory()
 
     QByteArray data;
     QDataStream stream( data, IO_WriteOnly );
-
-    stream << m_maxCount;   // not saved in history anymore, just a dummy here
-    stream << m_maxAgeDays; // not saved in history anymore, just a dummy here
 
     QPtrListIterator<KonqHistoryEntry> it( m_history );
     KonqHistoryEntry *entry;
Index: konq_historycomm.cc
===================================================================
RCS file: /home/kde/kdebase/libkonq/konq_historycomm.cc,v
retrieving revision 1.3
diff -u -3 -p -r1.3 konq_historycomm.cc
--- konq_historycomm.cc	16 Dec 2002 13:01:51 -0000	1.3
+++ konq_historycomm.cc	11 Jan 2003 17:13:26 -0000
@@ -1,11 +1,11 @@
 #include "konq_historycomm.h"
 
+bool KonqHistoryEntry::loadStringURL;
+
 // QDataStream operators (read and write a KonqHistoryEntry
 // from/into a QDataStream)
 QDataStream& operator<< (QDataStream& s, const KonqHistoryEntry& e) {
-    // TODO (BIC) stream the KURL, not just its .url(). Takes a bit more space,
-    // but prevents much reparsing when loading.
-    s << e.url.url();
+    s << e.url;
     s << e.typedURL;
     s << e.title;
     s << e.numberOfTimesVisited;
@@ -16,9 +16,16 @@ QDataStream& operator<< (QDataStream& s,
 }
 
 QDataStream& operator>> (QDataStream& s, KonqHistoryEntry& e) {
-    QString url;
-    s >> url;
-    e.url = url;
+    if (!KonqHistoryEntry::loadStringURL) //Backwards compatibility mode (V2 format)
+    {
+	s>>e.url;
+    }
+    else
+    {
+	QString url;
+	s >> url;
+	e.url = url;
+    }
     s >> e.typedURL;
     s >> e.title;
     s >> e.numberOfTimesVisited;
Index: konq_historycomm.h
===================================================================
RCS file: /home/kde/kdebase/libkonq/konq_historycomm.h,v
retrieving revision 1.4
diff -u -3 -p -r1.4 konq_historycomm.h
--- konq_historycomm.h	15 Jan 2001 15:28:07 -0000	1.4
+++ konq_historycomm.h	11 Jan 2003 17:13:26 -0000
@@ -28,6 +28,8 @@
 class KonqHistoryEntry
 {
 public:
+    //Backwards compatibility mode - set to true if loading V2 history files
+    static bool loadStringURL;
     KonqHistoryEntry()
 	: numberOfTimesVisited(1) {}
 

--Boundary-00=_yKFI+kOQJ/Z5DAs--