Closed Bug 1595625 Opened 4 months ago Closed 3 months ago

One directory object per address book

Categories

(MailNews Core :: Address Book, task)

task
Not set

Tracking

(thunderbird71 fixed, thunderbird72 fixed)

RESOLVED FIXED
Thunderbird 72.0
Tracking Status
thunderbird71 --- fixed
thunderbird72 --- fixed

People

(Reporter: darktrojan, Assigned: darktrojan)

Details

Attachments

(1 file, 1 obsolete file)

Every time we ask the address book manager for an address book by URI, a new AddrBookDirectory object is created. The manager caches these if there's no query in the URI, but if there is a query, it is not cached, so we create a new one. This is bad, especially since if we do a query we typically do many queries in a short time (e.g. finding a contact by typing).

I'm creating an AddrBookDirectoryInner object that is shared by all AddrBookDirectorys that reference the same book. Once we have that established we can use it to improve performance.

(Side objective: when we start changing APIs I want to abandon URIs with queries – actually I want to abandon URIs altogether but that's another topic – and do querying with a method on the single directory object per book.)

I'm not sure if this is clever use of javascript prototypes, or abuse of javascript prototypes, but it does work.

Attachment #9107968 - Flags: review?(mkmelin+mozilla)
Comment on attachment 9107968 [details] [diff] [review]
1595625-shared-addrbook-inner-1.diff

Found a bug.
Attachment #9107968 - Flags: review?(mkmelin+mozilla)

I realised that some things should be members of AddrBookDirectoryInner, not AddrBookDirectory. This was already a problem in theory but now we're explicitly stating there's one inner object it should be fixed.

I've created a member this._inner which is really the same thing as this.__proto__ but less confusing when it's referenced from the inner object itself.

Attachment #9107968 - Attachment is obsolete: true
Attachment #9107989 - Flags: review?(mkmelin+mozilla)
Comment on attachment 9107989 [details] [diff] [review]
1595625-shared-addrbook-inner-2.diff

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

LGTM, thx! r=mkmelin
Attachment #9107989 - Flags: review?(mkmelin+mozilla) → review+

Pushed by geoff@darktrojan.net:
https://hg.mozilla.org/comm-central/rev/18938b699f18
Create only one shared directory object per address book; r=mkmelin

Status: ASSIGNED → RESOLVED
Closed: 3 months ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 72.0
Attachment #9107989 - Flags: approval-comm-beta?
Attachment #9107989 - Flags: approval-comm-beta? → approval-comm-beta+
You need to log in before you can comment on or make changes to this bug.