Convert flags in nsIDocument into bit fields

RESOLVED FIXED in Firefox 42

Status

()

Core
DOM
RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: xidorn, Assigned: xidorn)

Tracking

Trunk
mozilla42
Points:
---

Firefox Tracking Flags

(firefox41 affected, firefox42 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

It seems there are 34 bool flags and 3 enum flags put continuously in nsIDocument. The 3 enum fields only need 3+3+2 bits. So in total, they only requires 42 bits or 6 bytes instead of 37 bytes there. As this is per-document, probably not very important, though.
Created attachment 8641546 [details]
MozReview Request: Bug 1164725 - Convert flags in nsIDocument into bit fields.

Bug 1164725 - Convert flags in nsIDocument into bit fields.
Attachment #8641546 - Flags: review?(bugs)
>   enum Type {
>     eUnknown, // should never be used
>     eHTML,
>     eXHTML,
>     eGenericXML,
>     eSVG,
>     eXUL
>   };
> 
>-  uint8_t mType;
>-
>-  uint8_t mDefaultElementType;
>-
>-  enum {
>+  Type mType : 3;
We don't usually use : <number> with enums. And this is error prone. What if more stuff is added to
Type enum and one forgets to update <number>.
So, please don't do this


+
+  enum Tri {
     eTriUnset = 0,
     eTriFalse,
     eTriTrue
   };
 
-  uint8_t mAllowXULXBL;
+  Tri mAllowXULXBL : 2;
Ditto.
Attachment #8641546 - Flags: review?(bugs) → review+
url:        https://hg.mozilla.org/integration/mozilla-inbound/rev/9daee31e3b9955740070a346c35a59b1fef5fd9e
changeset:  9daee31e3b9955740070a346c35a59b1fef5fd9e
user:       Xidorn Quan <quanxunzhen@gmail.com>
date:       Sat Aug 01 10:49:19 2015 +1000
description:
Bug 1164725 - Convert flags in nsIDocument into bit fields. r=smaug
(In reply to Olli Pettay [:smaug] from comment #2)
> We don't usually use : <number> with enums. And this is error prone. What if
> more stuff is added to
> Type enum and one forgets to update <number>.

I guess we could have a static analysis to check whether the length is large enough for an enum in the future.
It seems this commit improves the performance a little bit, which is detected by the talos test. I received an email:

Improvement: Mozilla-Inbound-Non-PGO - SVG-ASAP - Ubuntu HW 12.04 x64 - 2.1% decrease
-------------------------------------------------------------------------------------
    Previous: avg 271.546 stddev 1.962 of 12 runs up to revision 12d229711683
    New     : avg 265.843 stddev 0.864 of 12 runs since revision ffac74e3366c
    Change  : -5.703 (2.1% / z=2.907)
    Graph   : http://mzl.la/1KKh2J4

Changeset range: http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=12d229711683&tochange=ffac74e3366c

Changesets:
  * http://hg.mozilla.org/integration/mozilla-inbound/rev/9daee31e3b99
    : Xidorn Quan <quanxunzhen@gmail.com> - Bug 1164725 - Convert flags in nsIDocument into bit fields. r=smaug
    : http://bugzilla.mozilla.org/show_bug.cgi?id=1164725

  * http://hg.mozilla.org/integration/mozilla-inbound/rev/ffac74e3366c
    : Xidorn Quan <quanxunzhen@gmail.com> - Bug 1173930 - Not invoke FullscreenChange callback on OS X if state is not changed. r=smichaud
    : http://bugzilla.mozilla.org/show_bug.cgi?id=1173930

Bugs:
  * http://bugzilla.mozilla.org/show_bug.cgi?id=1173930 - Intermittent browser_fullscreen-window-open.js,browser_fxa_migrate.js | application crashed [@ nsGlobalWindow::FinishFullscreenChange(bool)] (Assertion failure: false (MOZ_ASSERT_UNREACHABLE: Failed to exit fullscreen?), at nsGlobalWindow.cpp:6173)
  * http://bugzilla.mozilla.org/show_bug.cgi?id=1164725 - Convert flags in nsIDocument into bit fields
https://hg.mozilla.org/mozilla-central/rev/9daee31e3b99
Assignee: nobody → quanxunzhen
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox42: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
You need to log in before you can comment on or make changes to this bug.