1242050 - C-C TB: use a default output stream buffer size of 16KB instead of 4KB

Status: RESOLVED FIXED

(MailNews Core :: Database, defect)

Description

[ISHIKAWA, Chiaki]

Attachment: buffer-by-16K-by-default.patch

I created a patch to make the default buffering to 16KB for
message copying. In this day and age, we can use 16KB
for output buffering without worrying about memory exhaustion.
(Already, mork uses 32KB buffering.)

Do note, though, the buffering is not effective until bugs below are patched.
[] Bug 1116055
[] Bug 1242030 - Consolidated patch set from bug 1122698, bug 1134527, bug 1134529, bug 1174500
[] Bug 1242042 - Enabling buffering for file stream to write message for C-C TB
[] Bug 1176857 - 1116055-acelist.patch: One liner patch posted in Bug 1116055 to return a buffered stream from mbox::getNewOutputStream by aceman
[] Bug 1242046:  	Removing unnecessary |Seek| that caused the C-C TB to operate slowly in terms of I/O 


I will post the roadmap of related patches shortly.

TIA

Comment 1

[ISHIKAWA, Chiaki]

The previous version of this memo was posted to
https://bugzilla.mozilla.org/show_bug.cgi?id=1116055#c101
on Jan 23, and then I got boggled down to fix Windows ONLY errors on
try-comm-server and could not follow up adequately.

Finally the strange errors observed only for Windows build on
try-comm-central have been fixed.  So here is the updated road-map for
this and other patches.  (Feb 2016 version)

========================================
PLANNED ORDER of landing patches
========================================

bug 1116055
  -> bug 1242030 (consolidation of 1122698, 1134527, 1134529, 1174500)
    -> bug 1242042 
       -> bug 1176857
         -> bug 1242046
           -> bug 1242050
             -> bug 1170606

========================================
An overview of patches in bugzilla entries.
----------------------------------------

[In February 2016, I fixed the errors observed ONLY UNDER WINDOWS when
we use Maildir format for storage. Thus I needed to tweak my patch set
somewhat and added a patch to take care of this issue and so I updated
this write-up.]

...

[10] New in 2016
1242050 - C-C TB: use a default output stream buffer size of 16KB instead of 4KB 
buffer-by-16K-by-default.patch: output buffer by 16KB by default

 I created a patch to make the default buffering to 16KB for message
 copying. In this day and age, we can use 16KB for output buffering
 without worrying about memory exhaustion.  (Already, mork uses 32KB
 buffering.)
 This should help when a local LAN uses a jumbo packet, too.

 This patch is theoretically independent of any of the other patches
 here.  But in practice, the buffering is not effective at all without
 the patches in [1]-[9] above.

I may want to land this patch first to help aceman to get ahead with his own patch to make this
setting of buffer size more flexible.

Comment 2

[ISHIKAWA, Chiaki]

Hi,

I would like to propose this patch to be included.
This is a very safe patch.
The submission on tryserver is as follows.
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=c7ded2eeb081&selectedJob=16980

I have kept this patch as part of my performance improvement (for downloading message copy to local folder), but since this buffering is used for all types of
buffered output (where buffering is effective), it should be proposed as an independent patch and I understand aceman is waiting to work on make this value a user-settable variable.

TIA

PS: The rest of performance improvement can wait for a while.

Comment 3

[Wayne Mery (:wsmwk)]

We should probably ask someone other than jcranmer to review this fairly trivial patch.

Comment 4

[:aceman]

Comment on attachment 8711188 [details] [diff] [review]
buffer-by-16K-by-default.patch

Review of attachment 8711188 [details] [diff] [review]:
-----------------------------------------------------------------

Yes, this should be safe. Also it can be landed independently of the other patches in the stack.

Comment 5

[:aceman]

https://hg.mozilla.org/comm-central/rev/5701162f52035f6b6d71ba849173ead3a498dd50