All users were logged out of Bugzilla on October 13th, 2018

Speed up cookie deleting in the cookie manager by batching

VERIFIED FIXED

Status

()

VERIFIED FIXED
15 years ago
14 years ago

People

(Reporter: mvl, Assigned: mvl)

Tracking

({perf})

Trunk
Points:
---
Bug Flags:
blocking1.6 -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

Wehen deleting multiple cookies from the cookiemanager, for every deletion the
entire list is loaded (and sorted) again. This is obviously very slow when
deleting lots of cookies.
Postponing the loading of the list will fix this. patch coming.
(Assignee)

Comment 1

15 years ago
Created attachment 136111 [details] [diff] [review]
Postpone loading until after all deletions
(Assignee)

Updated

15 years ago
Attachment #136111 - Flags: superreview?(darin)
Attachment #136111 - Flags: review?(neil.parkwaycc.co.uk)
Isn't this bug a dup of 210465 ?

Updated

15 years ago
Keywords: perf

Comment 3

15 years ago
Changing summary from "Deleting lots of cookie from the cookie mananager is
slow" to "Speed up cookie deleting in the cookie manager by batching".
Summary: Deleting lots of cookie from the cookie mananager is slow → Speed up cookie deleting in the cookie manager by batching

Comment 4

15 years ago
*** Bug 210465 has been marked as a duplicate of this bug. ***

Comment 5

15 years ago
If it's the actual sort that is slow, that should hopefully be improved by bug
224128.  But batching deletes before redisplaying is probably a good idea.

Comment 6

15 years ago
i like this patch... it's something i can incorporate into my
make-cookiemanager-not-suck patch as well :)

i don't think it's the sorting that's slow - it's just reloading the list, and
trying to do everything n times instead of once, that inherently makes it
irritating.

fwiw:
+  for (var c = deletedCookies.length - 1; c >= 0; c--) {

you can make this just:

   for (var c = deletedCookies.length; c--; ) {

Comment 7

15 years ago
Created attachment 136179 [details] [diff] [review]
Don't reload at all - we already delete the rows

Updated

15 years ago
Attachment #136179 - Flags: superreview?(darin)
Attachment #136179 - Flags: review?(dwitte)
(Assignee)

Comment 8

15 years ago
Comment on attachment 136179 [details] [diff] [review]
Don't reload at all - we already delete the rows

Yeah, this will work too. Thanks Neil.

Updated

15 years ago
Attachment #136179 - Flags: review?(dwitte) → review+
*** Bug 226625 has been marked as a duplicate of this bug. ***
Flags: blocking1.6?
(Assignee)

Updated

15 years ago
Attachment #136111 - Attachment is obsolete: true
Attachment #136111 - Flags: superreview?(darin)
Attachment #136111 - Flags: review?(neil.parkwaycc.co.uk)

Comment 10

15 years ago
think its too late for 1.6 on this.  a good one for 1.7
Flags: blocking1.6? → blocking1.6-

Comment 11

15 years ago
*** Bug 228541 has been marked as a duplicate of this bug. ***

Comment 12

15 years ago
*** Bug 228642 has been marked as a duplicate of this bug. ***
Blocks: 221185

Comment 13

15 years ago
mvl: Bug 228642 says this causes a full hang (not just slow) in 1.6 beta. Do you
think that is a dupe or a new problem?
(Assignee)

Comment 14

15 years ago
*** Bug 232152 has been marked as a duplicate of this bug. ***

Comment 15

15 years ago
Comment on attachment 136179 [details] [diff] [review]
Don't reload at all - we already delete the rows

sr=darin
Attachment #136179 - Flags: superreview?(darin) → superreview+

Comment 16

15 years ago
Fix checked in.
Status: NEW → RESOLVED
Last Resolved: 15 years ago
Resolution: --- → FIXED
*** Bug 238342 has been marked as a duplicate of this bug. ***
Verified FIXED using build 2004-07-25-09 on Windows XP.  This has been quite
zippy for a while now.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.