Boris Zbarsky [:bzbarsky] Assignee
	Description • 19 years ago

We have a lot of places that GetAttr just to do comparisons on the value.  These
comparisons could be optimized if they were happening inside nsAttrValue in a
lot of cases (with fallback to exactly the speed we have right now).

	Boris Zbarsky [:bzbarsky] Assignee
	Comment 1 • 19 years ago

I'll start converting callers in followup bugs once we're happy with the API.

In my testing, if the caller's type (string/atom) matches the callee's type for
the attr this is about 3 times faster than a GetAttr/compare, while if the
types don't match it's still about 30-40% faster.

	Johnny Stenback (:jst)
	Comment 2 • 19 years ago

Comment on attachment 195348 [details] [diff] [review]
Like so

   virtual PRBool HasAttr(PRInt32 aNameSpaceID, nsIAtom* aName) const = 0;
+  virtual PRBool AttrHasValue(PRInt32 aNameSpaceID,
+			       nsIAtom* aName,
+			       const nsAString& aValue,
+			       PRBool aCaseSensitive) const
+  virtual PRBool AttrHasValue(PRInt32 aNameSpaceID, nsIAtom* aName,
+			       nsIAtom* aValue, PRBool aCaseSensitive) const

I was sort of thinking about naming these HasAttrWithValue(), but I'm not sure
now which is better.

r+sr=jst either way.

	Boris Zbarsky [:bzbarsky] Assignee
	Comment 3 • 19 years ago

So... People suggested AttrValueIs or AttrIs.  Shaver also objected to having
PR_TRUE/PR_FALSE for the case and said separate methods would be better, but I
couldn't think of any reasonably short names for the case-insensitive one.

Using an enum instead of true/false was also suggested, but I really don't want
to include nsIContent in nsAttrValue.h, and declaring two separate enums that
just happen to share the same values seems fishy to me.

jst?  Thoughts?

	Boris Zbarsky [:bzbarsky] Assignee
	Comment 8 • 19 years ago

Fixed.  I'll file followup bugs on other places that can use these methods.

	Boris Zbarsky [:bzbarsky] Assignee
	Comment 9 • 19 years ago

Oh, and it looks like this checkin improved Tdhtml by 5-6%.