Currently, nscore.h [1] has NS_LIKELY and NS_UNLIKELY. MOZ_LIKELY and MOZ_UNLIKELY in mozilla/Likely.h [2] are better alternatives. The purpose of this bug is to * Replace all occurrences of NS_LIKELY and NS_UNLIKELY [3,4] with the MOZ_* variants; * Add #include "mozilla/Likely.h" to all touched files; * Remove the definitions of NS_LIKELY and NS_UNLIKELY from nscore.h Since there are a few hundred occurrences of these macros, it would probably be easiest to write a script to automate the first part. You could use [5] for inspiration. Feel free to ask me if you have any questions. [1] http://mxr.mozilla.org/mozilla-central/source/xpcom/base/nscore.h#366 [2] http://mxr.mozilla.org/mozilla-central/source/mfbt/Likely.h [3] http://mxr.mozilla.org/mozilla-central/ident?i=NS_LIKELY [4] http://mxr.mozilla.org/mozilla-central/ident?i=NS_UNLIKELY [5] https://bug579517.bugzilla.mozilla.org/attachment.cgi?id=653946
> MOZ_LIKELY and MOZ_UNLIKELY in mozilla/Likely.h [2] are better alternatives. No, they are worse alternatives. They fail to compile: if (MOZ_LIKELY(f)) // f is a nsIFrame* (see bug 340244 for details) I do agree with the naming change though, but please change the MOZ_ macros to match the NS_ ones first.
(In reply to Mats Palmgren [:mats] from comment #1) > > MOZ_LIKELY and MOZ_UNLIKELY in mozilla/Likely.h [2] are better alternatives. > > No, they are worse alternatives. They fail to compile: > if (MOZ_LIKELY(f)) // f is a nsIFrame* > > (see bug 340244 for details) > > I do agree with the naming change though, but please change the > MOZ_ macros to match the NS_ ones first. This has been fixed.
Hi Ms2ger, i would like to help on this bug. Could you please assist me with this ? Thank you.
(In reply to tienthanh8490 from comment #3) > Hi Ms2ger, i would like to help on this bug. Could you please assist me with > this ? Thank you. Certainly. If you aren't yet, it's probably easiest to get on IRC (<https://wiki.mozilla.org/IRC>), and join the #introduction channel.
tienthanh8490, are you still working on this bug? If not, I'd be happy to take it over
Created attachment 675285 [details] [diff] [review] Replace NS_{UN,}LIKELY with MOZ_{UN,}LIKELY across tree It was a slow day today, so I figured.. And it even builds.
(In reply to chronon from comment #5) > tienthanh8490, are you still working on this bug? If not, I'd be happy to > take it over Yes i dropped out, please take it over
Comment on attachment 675285 [details] [diff] [review] Replace NS_{UN,}LIKELY with MOZ_{UN,}LIKELY across tree Review of attachment 675285 [details] [diff] [review]: ----------------------------------------------------------------- Thanks a lot! I made some small comments below; please fix those and ask :ehsan to review. Henri, could you check if the changes here touch generated code? ::: accessible/src/atk/nsMaiInterfaceAction.cpp @@ +8,5 @@ > > #include "Accessible-inl.h" > #include "nsMai.h" > #include "Role.h" > +#include "mozilla/Likely.h" Remove the extra space at the end of this line, here and elsewhere in the patch. ::: content/base/src/nsINode.cpp @@ +14,5 @@ > #include "mozAutoDocUpdate.h" > #include "mozilla/CORSMode.h" > #include "mozilla/Telemetry.h" > #include "mozilla/Util.h" > +#include "mozilla/Likely.h" Put this one between CORSMode and Telemetry, to preserve the alphabetical ordering. ::: gfx/thebes/gfxFont.cpp @@ +33,5 @@ > #include "mozilla/FloatingPoint.h" > #include "mozilla/Preferences.h" > #include "mozilla/Services.h" > #include "mozilla/Telemetry.h" > +#include "mozilla/Likely.h" Ordering again. ::: gfx/thebes/gfxPlatformFontList.cpp @@ +17,5 @@ > #include "mozilla/Preferences.h" > #include "mozilla/Telemetry.h" > #include "mozilla/TimeStamp.h" > #include "mozilla/Attributes.h" > +#include "mozilla/Likely.h" Sort those too ::: js/xpconnect/src/nsXPConnect.cpp @@ +8,5 @@ > > #include "mozilla/Assertions.h" > #include "mozilla/Base64.h" > #include "mozilla/Util.h" > +#include "mozilla/Likely.h" Again. ::: layout/base/nsPresShell.cpp @@ +20,5 @@ > > #include "mozilla/dom/PBrowserChild.h" > #include "mozilla/dom/TabChild.h" > #include "mozilla/Util.h" > +#include "mozilla/Likely.h" Here ::: layout/generic/nsTextFrameThebes.cpp @@ +73,5 @@ > #include "mozilla/dom/Element.h" > #include "mozilla/Util.h" // for DebugOnly > #include "mozilla/LookAndFeel.h" > #include "mozilla/Attributes.h" > +#include "mozilla/Likely.h" Sort those too, please. ::: layout/style/nsRuleNode.cpp @@ +43,5 @@ > #include "mozilla/Assertions.h" > #include "mozilla/dom/Element.h" > #include "mozilla/LookAndFeel.h" > #include "mozilla/Util.h" > +#include "mozilla/Likely.h" Sort ::: layout/xul/base/src/nsMenuFrame.cpp @@ +40,5 @@ > #include "mozilla/Services.h" > #include "mozilla/Preferences.h" > #include "mozilla/LookAndFeel.h" > #include "mozilla/Attributes.h" > +#include "mozilla/Likely.h" And here, please. ::: parser/html/nsHtml5TreeOpExecutor.cpp @@ +571,1 @@ > NS_ERROR_HTMLPARSER_INTERRUPTED)) { Fix the indentation here ::: toolkit/xre/nsAppRunner.cpp @@ +1061,5 @@ > #endif > > #define NS_ENSURE_TRUE_LOG(x, ret) \ > PR_BEGIN_MACRO \ > + if (MOZ_UNLIKELY(!(x))) { \ Fix the alignment of the backslash ::: xpcom/base/nsCycleCollector.cpp @@ +140,5 @@ > #include "mozilla/Mutex.h" > #include "mozilla/CondVar.h" > #include "mozilla/StandardInteger.h" > #include "mozilla/Telemetry.h" > +#include "mozilla/Likely.h" Sort, please
Created attachment 675534 [details] [diff] [review] Fixed ordering, indentation, and removed trailing whitespaces
Oops, shouldn't have marked the previous one as obsolete, I guess - looks like I cleared hsivonen's review? flag..? Sorry.
r+ for the generated parser files from me. There are indeed generated files being edited, but I can easily change the generator to match once this has landed.
I'll try to review this over the weekend. Sorry for the delay!
Comment on attachment 675534 [details] [diff] [review] Fixed ordering, indentation, and removed trailing whitespaces Review of attachment 675534 [details] [diff] [review]: ----------------------------------------------------------------- Looks great, thanks! ::: layout/style/nsRuleNode.cpp @@ +2320,5 @@ > else \ > data_ = new (mPresContext) nsStyle##type_ ctorargs_; \ > } \ > \ > + if (MOZ_UNLIKELY(!data_)) \ There's a bunch of places where this patch breaks indentation but I can fix those when landing.
https://hg.mozilla.org/integration/mozilla-inbound/rev/7bd96dda75f0
https://hg.mozilla.org/mozilla-central/rev/7bd96dda75f0
or
