Closed Bug 707946 Opened 8 years ago Closed 8 years ago

Speed-up hasChildren for tags

Categories

(Toolkit :: Places, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla11

People

(Reporter: mak, Assigned: mak)

References

Details

(Keywords: addon-compat, perf)

Attachments

(1 file)

We already know a tag has children and the tagging service has a nice cache we can leverage.
Attached patch patch v1.0Splinter Review
Will require SR but it's a small change
Attachment #579486 - Flags: review?(dietrich)
Comment on attachment 579486 [details] [diff] [review]
patch v1.0

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

r=me. could this cause the tag service to initialize earlier than previously? if so, should keep an eye on Ts.
Attachment #579486 - Flags: review?(dietrich) → review+
Only if you have a copy of the Tags query in your bookmarks toolbar and the toolbar is visible. Otherwise that code path is hit when opening the Library.
Comment on attachment 579486 [details] [diff] [review]
patch v1.0

Needs SR for the attribute addition.
Note that using allTags.length wouldn't be as performant since allTags does an expensive sorting to return tags sorted by name.

I expect in future we may just merge this interface into the bookmarks interface, since we allow to tag bookmarks and having them separated caused more troubles than wins.
Attachment #579486 - Flags: superreview?(gavin.sharp)
Comment on attachment 579486 [details] [diff] [review]
patch v1.0

>diff --git a/toolkit/components/places/nsITaggingService.idl b/toolkit/components/places/nsITaggingService.idl

> interface nsITaggingService : nsISupports

>+  /**
>+   * Whether there is any tag.
>+   */
>+  readonly attribute boolean hasTags;

"hasTags" seems a little awkward but I can't come up with anything better. I'd change the comment to "whether any tags exist", though.
Attachment #579486 - Flags: superreview?(gavin.sharp) → superreview+
fixed the comment, I also added a brief @note regarding the performance difference
https://hg.mozilla.org/integration/mozilla-inbound/rev/1b51ee2ee388
Keywords: addon-compat
Target Milestone: --- → mozilla11
https://hg.mozilla.org/mozilla-central/rev/1b51ee2ee388
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Blocks: 492805
You need to log in before you can comment on or make changes to this bug.