Closed Bug 1041836 Opened 7 years ago Closed 7 years ago

Drop redundant field initialisers

Categories

(Firefox for Android Graveyard :: General, defect)

All
Android
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 34

People

(Reporter: ckitching, Assigned: ckitching)

References

Details

Attachments

(1 file)

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".
Component: Overlays → General
No longer blocks: fatfennec
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+
(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!
https://hg.mozilla.org/mozilla-central/rev/2b5e76962106
Assignee: nobody → chriskitching
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 34
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.