Closed Bug 1041836 Opened 7 years ago Closed 7 years ago
Drop redundant field initialisers
Another weekend project in the field of automated code audit. (Sorry in advance for the dull review, RNewman). In Java, fields of numeric types are guaranteed to be initialised to zero before use. Object types are similarly initialised to null. So if we have some code like: private int someField = 0; This is semantically identical to: private int someField; Unfortunately, javac is stupid. If you write code with the explicit initialiser it generates bytecode for the assignment and copies it into every constructor (five bytes of bytecode per constructor per field). Handily, I still have that static analysis framework lying around, so I used it to generate a patch to remove all such redundant initialisers from fennec. I then manually audited the results and had it ignore a few that were serving as error codes and whatnot in the name of clarity. Doing this seems to save us something around 12kb of apk size, so seems to be vaguely worthwhile. If this seems like something you want, I'll split the patch up for merging with sync and so forth.
Attachment #8459917 - Flags: review?(rnewman)
I should also add: This is an optimisation Proguard does not do. Booleans are default-initialised to "false".
Hardware: ARM → All
Summary: Drop redundant field initialisers. → Drop redundant field initialisers
Comment on attachment 8459917 [details] [diff] [review] Remove redundant field initialisers. Review of attachment 8459917 [details] [diff] [review]: ----------------------------------------------------------------- It's disappointing that javac can't figure this out. Please also do this on a branch in https://github.com/mozilla-services/android-sync. We'll need to land that separately. The inline comment applies to android-sync. ::: mobile/android/base/background/healthreport/Environment.java @@ -69,5 @@ > > /** > * One of the Configuration#SCREENLAYOUT_SIZE_* constants. > */ > - public int screenLayout = 0; // SCREENLAYOUT_SIZE_UNDEFINED = 0 Leave the values in this file -- indeed, default it to Configuration.SCREENLAYOUT_SIZE_UNDEFINED, not zero.
Attachment #8459917 - Flags: review?(rnewman) → review+
Any chance this patch caused this failure? https://tbpl.mozilla.org/php/getParsedLog.php?id=44646358&tree=Fx-Team
(In reply to Nigel Babu [:nigelb] from comment #4) > Any chance this patch caused this failure? > https://tbpl.mozilla.org/php/getParsedLog.php?id=44646358&tree=Fx-Team Zero chance. This is a Java-only patch.
Status: NEW → ASSIGNED
Heh, I figured that out when I read the patch later. Besides, restarts seem to have fixed the failure. Cheers!
Assignee: nobody → chriskitching
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 34
This needed to land in android-sync. I've backported: https://github.com/mozilla-services/android-sync/commit/fd0c83fbc20427336f6aa3309325045c830b1816
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.