Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mailbox/listing: Make server response for large mailbox listing faster (threaded view) #5194

Merged

Conversation

bostjan
Copy link
Contributor

@bostjan bostjan commented Apr 9, 2016

Symptom

When using roundcube with mailboxes with over 60k messages, list
view was way faster than viewing in threaded view.

Mailbox index view timing: ~360 ms
Mailbox threaded view timing: ~800 ms

Resolution

Use native PHP array manipulation functions instead of rolling custom
string data reversal implementation using strpos() and substr() in a
'while' loop.

This optimization is already present in index view handler, but was missing
from threaded view.

Results after optimization

Both average out around ~360 ms response time.

b.

PS: Could this be merged into 1.1 branch also? Tnx.

…r when using threaded view

Symptom
=======
When using roundcube with mailboxes with over 60k messages, list
view was way faster than viewing in threaded view.

Mailbox index view timing:    ~360 ms
Mailbox threaded view timing: ~800 ms

Resolution
==========
Use native PHP array manipulation functions instead of rolling custom
string data reversal implementation using strpos() and substr() in a
'while' loop.

This optimization is already present in index view handler, but was missing
from threaded view.

Results after optimization
==========================
Both average out around ~360 ms response time.
@alecpl
Copy link
Member

alecpl commented Apr 10, 2016

Problem is that this consumes much more memory. Did you check that? Maybe on PHP7 it looks reasonable, but on older versions big array is a memory killer.

@alecpl
Copy link
Member

alecpl commented Apr 10, 2016

Hmm... it looks in case of the revert() method the memory use is not so bad. For ~100k elements in a thread the old code uses 2MB, the new one 6MB. I think we can accept that, especially that indeed rcube_thread_index uses the same, array-based method. At the same time it looks like the new code is 300x faster ;)

@alecpl alecpl merged commit 463d078 into roundcube:master Apr 10, 2016
@bostjan
Copy link
Contributor Author

bostjan commented Apr 10, 2016

Nope, I did not check that, it did not occur to me as a potential issue, tnx for reminder.
Unfortunately due to RC 1.1.x requirements I am stuck with PHP 5.6 and did not even think about 7.0 here.

I did a little test to compare 5.6 and 7.0 memory usage, code and results are below. I think this is a nice tradeoff, especially as this memory is immediately released back to PHP for reuse.

If this turns out to be a potential source of problems (mem wise), we might refactor this to configurable option - faster & more memory, or slower & less memory required. But util someone complains, no thought should be given to this.

7.0:

(0)[root@labrat-2:~] 20:51:30
# php -v
PHP 7.0.5 (cli) (built: Apr  6 2016 12:29:53) ( NTS )

(0)[root@labrat-2:~] 20:52:49
# php test.php 
Peak PHP: 384.42 K   Peak Real: 2 M   CurPHP:348.79 K   CurReal:2 M
Peak PHP: 4.34 M   Peak Real: 6 M   CurPHP:4.34 M   CurReal:6 M
Peak PHP: 10.34 M   Peak Real: 12.01 M   CurPHP:4.81 M   CurReal:8 M
Peak PHP: 10.34 M   Peak Real: 12.01 M   CurPHP:4.34 M   CurReal:8 M
Peak PHP: 10.34 M   Peak Real: 12.01 M   CurPHP:348.82 K   CurReal:4 M

5.6:

(0)[root@labrat-1:~] 20:58:23
# php -v
PHP 5.6.20 (cli) (built: Apr  6 2016 11:44:13) 
Copyright (c) 1997-2016 The PHP Group
Zend Engine v2.6.0, Copyright (c) 1998-2016 Zend Technologies

(0)[root@labrat-1:~] 20:58:25
# php test.php 
Peak PHP: 232.38 K   Peak Real: 256 K   CurPHP:226.2 K   CurReal:256 K
Peak PHP: 14.19 M   Peak Real: 14.5 M   CurPHP:14.19 M   CurReal:14.5 M
Peak PHP: 24.05 M   Peak Real: 24.75 M   CurPHP:14.66 M   CurReal:15.25 M
Peak PHP: 24.05 M   Peak Real: 24.75 M   CurPHP:14.19 M   CurReal:14.75 M
Peak PHP: 24.05 M   Peak Real: 24.75 M   CurPHP:226.45 K   CurReal:768 K

5.5:

bostjan@agent-shepherd-ubuntu:~$ php -v
PHP 5.5.9-1ubuntu4.14 (cli) (built: Oct 28 2015 01:34:46) 

bostjan@agent-shepherd-ubuntu:~$ php test.php 
Peak PHP: 238.45 K   Peak Real: 256 K   CurPHP:232.55 K   CurReal:256 K
Peak PHP: 14.2 M   Peak Real: 14.5 M   CurPHP:14.2 M   CurReal:14.5 M
Peak PHP: 24.06 M   Peak Real: 24.75 M   CurPHP:14.66 M   CurReal:15.25 M
Peak PHP: 24.06 M   Peak Real: 24.75 M   CurPHP:14.2 M   CurReal:14.75 M
Peak PHP: 24.06 M   Peak Real: 24.75 M   CurPHP:232.77 K   CurReal:768 K

Code used:

<?php



function formatBytes($size, $precision = 2)
{
    $base = log($size, 1024);
    $suffixes = array('', 'K', 'M', 'G', 'T');   

    return round(pow(1024, $base - floor($base)), $precision) .' '. $suffixes[floor($base)];
}



function memInfo () {
    $memCurPhp   = memory_get_usage(false);
    $memCurReal  = memory_get_usage(true);
    $memPeakPhp  = memory_get_peak_usage(false);
    $memPeakReal = memory_get_peak_usage(true);
    echo "Peak PHP: ". formatBytes($memPeakPhp) ."   Peak Real: ". formatBytes($memPeakReal) ."   CurPHP:". formatBytes($memCurPhp) ."   CurReal:". formatBytes($memCurReal) ."\n";
}



memInfo();
$data = array();

for ($i=0 ; $i<100000 ; $i++) {
    $data[] = $i;
}
memInfo();

//explode(self::SEPARATOR_ELEMENT, $this->raw_data);
$dataReverse = implode('', array_reverse($data));
memInfo();

unset($dataReverse);
memInfo();

unset($data);
memInfo();

@bostjan
Copy link
Contributor Author

bostjan commented Apr 13, 2016

I recieved some emails from Kolab about builds failing and this commit being mentioned in there - who is the contact person for that?

b.

@alecpl
Copy link
Member

alecpl commented Apr 13, 2016

You can ignore that. Their build may fail for many reasons. These notifications are sent automatically by phabricator. Or you're asking to not receive such notifications? Then I could ask someone.

@bostjan
Copy link
Contributor Author

bostjan commented Apr 13, 2016

Thanks for explanation.

It's fine, I will adjust email notification settings myself.

b.

@alecpl
Copy link
Member

alecpl commented Apr 13, 2016

Oh, I just noticed that you have an account in Kolab's phabricator already, so indeed not a bug ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants