1658128 - AddrBookDirectory.childCards and AddrBookDirectory.search do more work than necessary

Status: RESOLVED FIXED

(MailNews Core :: Address Book, defect)

Description

[Mark Banner (:standard8)]

AddrBookDirectory.childCards and AddrBookDirectory.search both do more work than necessary (I think some of the code could be shared between them, but that's a different bug).

I've noticed two things, the first:

1. The initially get the full set of card results from the stored list of cards, and then get a card object for each contact which is fed into an array in results variable.
2. The cards in results are then filtered using an array filter.
3. At this stage, the code goes and obtains a second list of properties for the card, rather than using the properties that were obtained as part of step 1.

This seems inefficient as it is cloning property maps more than necessary.

The second:

1. In processing the query, the functions split it up into an array tree first.
2. The array tree is then iterated over.
3. Every time we hit a condition element (a string), it is split and then the value is decoded and lower cased.

This third case can be hit many times - in the case of quickSearch from the extension API, I think it is 13 times per card for each card in the database (assuming the contact isn't matched).

We should be able to move those into step 1 and avoid the additional overhead.

Both of these were showing up in performance profiles of Conversations which will sometimes do multiple queries on the address book.

Comment 1

[Mark Banner (:standard8)]

Attachment: Bug 1658128 - Avoid extra copying of properties maps and splitting of condition strings in address book searches. r?darktrojan!

Depends on D86474

Comment 2

[Mark Banner (:standard8)]

Requesting tracking as this affects regular searches and ones from extensions.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/b736b95d2018
Avoid extra copying of properties maps and splitting of condition strings in address book searches. r=darktrojan

Comment 4

[Mark Banner (:standard8)]

Comment on attachment 9168988 [details]
Bug 1658128 - Avoid extra copying of properties maps and splitting of condition strings in address book searches. r?darktrojan!

[Approval Request Comment]
Regression caused by (bug #): Address book changes.
User impact if declined: Performance impact, functionality using quickSearch/Search may be slow.
Testing completed (on c-c, etc.): Landed on c-c.
Risk to taking this patch (and alternatives if risky): Should be low - will be fairly obvious if the function is broken.

Comment 5

[Wayne Mery (:wsmwk)]

Comment on attachment 9168988 [details]
Bug 1658128 - Avoid extra copying of properties maps and splitting of condition strings in address book searches. r?darktrojan!

[Triage Comment]
Approved for beta

Comment 6

[Rob Lemley [:rjl]]

Thunderbird 80.0b3:
https://hg.mozilla.org/releases/comm-beta/rev/62eeb4c4933e

Comment 7

[Wayne Mery (:wsmwk)]

Comment on attachment 9168988 [details]
Bug 1658128 - Avoid extra copying of properties maps and splitting of condition strings in address book searches. r?darktrojan!

[Triage Comment]
Approved for esr78

Comment 8

[Rob Lemley [:rjl]]

Thunderbird 78.1.2:
https://hg.mozilla.org/releases/comm-esr78/rev/fa27a91d55a8