nsIAtom.Equals(AString) involves a string copy plus conversion to do the comparison. I think it would be better to have a comparator that goes along and does on-the-fly comparison... This would probably mean factoring the conversion code out of ConvertUTF8toUTF16, which already lives in XPCOM. I know jst has mentioned this idea on and off, and I could really use it about now, what with bug 307600 (where we're possibly doing a bunch of direct string-to-atom compares).
Mmmm, factoring. If C++ doesn't help, use a C preprocessor macro! Ok, I'm old.... ;-) /be
I thought that jst (or bryner?) had code for this already and found that it made no measurable improvement. Am I dreaming?
Not sure... I did check for bugs on this and didn't see any... If there's a patch floating about, I would be happy to profile it on the testcases I have here.
I had written some code to do a non-copying comparison of UTF-8 to UTF-16 that could be used here. I didn't measure any speedup. Unfortunately, I'm fairly certain that I've lost that patch, unless jst has it.
See attachment 172366 [details] [diff] [review] in bug 277479 for jst's CompareUTF8toUTF16. Not sure why that never got checked in, iirc it was a nice speedup.
note also bug 231782
Bug 277479 was not checked in because there were no clients. As soon as there is a client which wants to use that code, let's get it landed!
(In reply to comment #7) > Bug 277479 was not checked in because there were no clients. As soon as there is > a client which wants to use that code, let's get it landed! https://bugzilla.mozilla.org/show_bug.cgi?id=277479#c14 quotes improvements for atom creation from char*, nsAString& and PRUnichar*. Is no one creating atoms anymore? ;-)
(In reply to comment #4) > I had written some code to do a non-copying comparison of UTF-8 to UTF-16 that > could be used here. I didn't measure any speedup. Unfortunately, I'm fairly > certain that I've lost that patch, unless jst has it. Bryner, I've still got that patch, it's part of attachment 172366 [details] [diff] [review] in bug 277479.
OK, I just tried that attachment on a simple testcase -- document.images['imageid'] for a document with a bunch of images. The results are: Without patch: 70098 hits under nsContentList::NamedItem, of which 21739 are under AtomImpl::Equals With patch: 59130 hits under nsContentList::NamedItem, of which 9822 are under AtomImpl::Equals So that patch makes AtomImpl::Equals over two times faster, and makes this quite real-life testcase about 15% faster. Given that, I think we should just get that patch in on the trunk, either in this bug or in bug 277479. Who would need to review it?
12 years ago
12 years ago
Fixed by patch for bug 314465.