nsMsgThreadedDBView::AddKeys does not preallocate enough space

RESOLVED WORKSFORME

Status

MailNews Core
Backend
RESOLVED WORKSFORME
16 years ago
8 years ago

People

(Reporter: David Einstein, Unassigned)

Tracking

(Blocks: 1 bug, {perf})

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [invalid?])

Attachments

(1 obsolete attachment)

(Reporter)

Description

16 years ago
This is an addendum to bug 74955 spawned out of bug 107369.
The problem is that AddKeys preallocates the space for a numbe of headers equal
to the number of threads.  If we are expanding the threads, this allocates far
to little space and we spend time reallocating.  We should allocate space for
the number of headers that we expect to store.
Patch to follow
(Reporter)

Comment 1

16 years ago
Created attachment 57804 [details] [diff] [review]
Allocate the right amount of space

This patch assumes that the patch for bug 103769 ( attachment 57693 [details] [diff] [review] or
equvalent ) has already been added.
Keywords: patch, review
QA Contact: esther → stephend
Hardware: PC → All

Comment 2

16 years ago
David or Cavin, is this patch something that should be taken?  Does it improve
the expanded case without slowing down the collapsed case?
Status: NEW → ASSIGNED
(Reporter)

Comment 3

16 years ago
The patch should not slow down the collapsed case, it only adds a branch and an
extra parameter to the function call to AddKeys.  In the non collapsed case it
should cut down the number of allocations.  Exactly how much depends on the
amount of threading.  I would not make it a high priority, but it should not
cause any harm either.

Comment 4

16 years ago
I think it's a low priority one and I don't think it would hurt the performance
in either case. For the non-collapsed case, my guess is that the improvement
will be very little for the most cases.

Comment 5

16 years ago
moving to 1.2.  If you think we should take it right away, move it back.  If you
think we shouldn't take it then change to Wontfix.  It sounds like you don't
think this will have much impact.
Target Milestone: --- → mozilla1.2

Updated

15 years ago
Attachment #57804 - Flags: superreview?(sspitzer)
Attachment #57804 - Flags: review?(sspitzer)

Comment 6

15 years ago
Comment on attachment 57804 [details] [diff] [review]
Allocate the right amount of space

This would probably get a faster review if it was a unified diff.

Comment 7

15 years ago
Comment on attachment 57804 [details] [diff] [review]
Allocate the right amount of space






fwiw comment #1 meant bug 107369
I applied the patch, cvs updated and then diffed and there weren't any
interesting changes left
Attachment #57804 - Attachment is obsolete: true
Attachment #57804 - Flags: superreview?(sspitzer)
Attachment #57804 - Flags: review?(sspitzer)

Updated

14 years ago
Blocks: 236849
Product: MailNews → Core

Updated

9 years ago
Assignee: cavin → nobody
Status: ASSIGNED → NEW
QA Contact: stephend → backend
(Assignee)

Updated

9 years ago
Product: Core → MailNews Core
Keywords: perf
(In reply to comment #7)
> (From update of attachment 57804 [details] [diff] [review])
 > fwiw comment #1 meant bug 107369
> I applied the patch, cvs updated and then diffed and there weren't any
> interesting changes left

Shall we close this bug then ?
Target Milestone: mozilla1.2alpha → ---

Comment 9

8 years ago
bienvenu?  

(In reply to comment #0)
> This is an addendum to bug 74955 spawned out of bug 107369.
> The problem is that AddKeys preallocates the space for a numbe of headers equal
> to the number of threads.  If we are expanding the threads, this allocates far
> to little space and we spend time reallocating.  We should allocate space for
> the number of headers that we expect to store.
> Patch to follow
Whiteboard: [invalid?]

Comment 10

8 years ago
sure, marking wfm.
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.