1503094 - Remove unused LOGTAG

Status: RESOLVED WONTFIX

(Firefox for Android Graveyard :: General, enhancement, P5)

Comment 1

[Andrew Gaul]

Attachment: Remove unused LOGTAG

Found via error-prone.  Removes 1 KB from apk.

Comment 2

[Jan Henning [:JanH]]

(In reply to Andrew Gaul from comment #1)
> Found via error-prone.  Removes 1 KB from apk.

Was that measured with an official build? For non-local/non-developer builds, I would have thought that Proguard would eliminate unused variables anyway, whereas local developers builds normally aren't proguarded.

The flip side is that this makes debugging slightly easier because you don't have to invent a LOGTAG yourself.
If this affects shipping builds as well, I withdraw my objections, though, even if it is only 1 kB.

Comment 3

[Nick Alexander :nalexander [he/him]]

Comment on attachment 9021012 [details] [diff] [review]
Remove unused LOGTAG

Fine by me.

Comment 4

[Andrew Gaul]

:JanH I only tested with mach build / mach package; do I need to do anything else to run proguard?  Some of these final fields were not constants since they invoked Class.getSimpleName so would proguard remove them?

Comment 5

[Jan Henning [:JanH]]

(In reply to Andrew Gaul from comment #4)
> :JanH I only tested with mach build / mach package; do I need to do anything
> else to run proguard?

By building with "export MOZILLA_OFFICIAL=1" in your mozconfig or by temporarily changing https://dxr.mozilla.org/mozilla-central/rev/c291143e24019097d087f9307e59b49facaf90cb/mobile/android/app/build.gradle#82-84, so that proguard runs on normal developer builds as well. But...

> Some of these final fields were not constants since
> they invoked Class.getSimpleName so would proguard remove them?

... I'd forgotten that we have those as well.
In that case, feel free to go ahead.

Comment 6

[Andrew Gaul]

I remeasured with official (with Proguard) builds, saving only 100 bytes:

 26765695  37096 -rw-rw-r--   1  gaul     gaul     37984331 Oct 30 11:23 ./objdir-frontend/dist/fennec-65.0a1.en-US.android-i686.apk
 26765103  37096 -rw-rw-r--   1  gaul     gaul     37984431 Oct 30 11:55 ./objdir-frontend/dist/fennec-65.0a1.en-US.android-i686.apk

I suspect this is due to removing Class.getSimpleName calls.  I could respin this patch to change the LOGTAG with non-constant data to constant.  What do you think?

Comment 7

[Jan Henning [:JanH]]

(In reply to Andrew Gaul from comment #6)
> I could respin  this patch to change the LOGTAG with non-constant data to constant.
> What do you think?

No objections to that of course.

But equally I have to admit that having to create your own LOGTAG should it ever be necessary to do some debugging in one of those classes of course isn't a dramatic hardship either, so in the end I could have lived with the original patch as well.

Comment 8

[Andrew Gaul]

If there are no objections I would like to merge this as-is and will simplify the existing and used LOGTAGs calling getSimpleName in a subsequent patch.

Comment 9

[BugBot [:suhaib / :marco/ :calixte]]

There's a r+ patch which didn't land and no activity in this bug for 2 weeks.
:andrew, could you have a look please?

Comment 10

[Andrew Gaul]

I will mark this as invalid since it has bitrotted and removing Class.getSimpleName calls seems more fruitful.