Closed Bug 110121 Opened 23 years ago Closed 15 years ago

nsMsgThreadedDBView::AddKeys does not preallocate enough space

Categories

(MailNews Core :: Backend, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED WORKSFORME

People

(Reporter: Deinst, Unassigned)

References

(Blocks 1 open bug)

Details

(Keywords: perf, Whiteboard: [invalid?])

Attachments

(1 obsolete file)

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
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
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
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.
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.
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
Attachment #57804 - Flags: superreview?(sspitzer)
Attachment #57804 - Flags: review?(sspitzer)
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 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)
Blocks: 236849
Product: MailNews → Core
Assignee: cavin → nobody
Status: ASSIGNED → NEW
QA Contact: stephend → backend
Product: Core → MailNews Core
(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 → ---
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?]
sure, marking wfm.
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: