1041836 - Drop redundant field initialisers

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Chris Kitching [:ckitching]]

Attachment: Remove 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.

Comment 1

[Chris Kitching [:ckitching]]

I should also add:

This is an optimisation Proguard does not do.
Booleans are default-initialised to "false".

Comment 2

[Richard Newman [:rnewman]]

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.

Comment 3

[Chris Kitching [:ckitching]]

https://hg.mozilla.org/integration/fx-team/rev/2b5e76962106

Comment 4

[Nigel Babu [:nigelb]]

Any chance this patch caused this failure? https://tbpl.mozilla.org/php/getParsedLog.php?id=44646358&tree=Fx-Team

Comment 5

[Richard Newman [:rnewman]]

(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.

Comment 6

[Nigel Babu [:nigelb]]

Heh, I figured that out when I read the patch later. Besides, restarts seem to have fixed the failure. Cheers!

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/2b5e76962106

Comment 8

[Nick Alexander :nalexander [he/him]]

This needed to land in android-sync.  I've backported:

https://github.com/mozilla-services/android-sync/commit/fd0c83fbc20427336f6aa3309325045c830b1816