Project

General

Profile

Bug #304

Bulk insert (+ a few simple regexp improvements)

Added by sph over 11 years ago. Updated about 11 years ago.

Status:
Resolved
Priority:
High
Assignee:
Category:
Quassel Client
Target version:
Start date:
Due date:
% Done:

0%

Estimated time:
Version:
0.13.0
OS:
Any

Description

This gives an enormous performance boost when requesting more backlog (ie. by scrolling up). The startup time remained pretty much the same for me though and I'm not sure why exactly.

My C++ coding might still be a bit rusty, but I hope it saves some of your time at least. I think it's pretty clean :)

bulkinsert.diff (7.18 KB) bulkinsert.diff admin, 08/30/2008 03:24 AM
bulkinsert2.diff (3.78 KB) bulkinsert2.diff admin, 08/30/2008 11:03 PM
chatview.diff (874 Bytes) chatview.diff admin, 08/31/2008 06:51 PM
chatline.diff (724 Bytes) chatline.diff admin, 08/31/2008 06:51 PM

History

#1 Updated by EgS over 11 years ago

I love the concept, but it is sadly faulty... The problem is, that it's not possible to insert a bulk of messages of one buffer into the message model, by just taking that bunch and insert it where the first (or last) message of that bunch belongs.

The reason behind this: the message model is sorted by message Id and message Ids are unique for a whole core (not even only a user). So imagine 2 buffers A, B receiving messages:

Buffer A:
10:30 A: msgA1
10:35 A: msgA2

Buffer B:
10:32 B: msgB1
10:37 B: msgB2

That would result in msgIds:
1 - > msgA1
2 -> msgB1
3 -> msgA2
4 -> msgB2

So the way your patch handles the bulk inserts would destroy the order. This is at the time being not a visible issue, but is crucial for some future things like logically merged buffers or things similar to the chat monitor that show chat from a configured bunch of channels.

But I think it's definitely something we can work with, I have some Ideas to fix that order issue without going back to single inserts and also some tweaks.

Thanks for investigating!

#2 Updated by sph over 11 years ago

The bulkinsert works properly now :)

Some benchmarks: http://lithitux.org/~sphilips/quassel/benchmark.txt

It won't make a difference until the code in qtuimessageprocessor calls insertMessages() instead of insertMessage() (which I am doing in the benchmarks)

(New diff: bulkinsert2.diff)

#3 Updated by sph over 11 years ago

Woops, small bug with huge impact:

In the foreach in function insertMessages(const QList<Message> &msglist):
idx += indexForId(msg.msgId()); // Wrong

Must be replaced by:
idx = indexForId(msg.msgId()); // Correct

No idea why I didn't spot that earlier :)

#4 Updated by sph over 11 years ago

EgS: I've added the paint tweaks too again (chatview.diff and chatline.diff)

I can't really tell how effective they are as I haven't done any worthy measurements, it's more from reading the docs and googling.

#5 Updated by Sputnick over 11 years ago

Incorporated meanwhile.

Also available in: Atom PDF