Closed Bug 963873 Opened 8 years ago Closed 6 years ago

[Linter: UseSparseArrays] Consider using SparseArray for HashMap in AndroidGamepadManager.java

Categories

(Firefox for Android Graveyard :: General, defect)

All
Android
defect
Not set
normal

Tracking

(firefox41 fixed)

RESOLVED FIXED
Firefox 41
Tracking Status
firefox41 --- fixed

People

(Reporter: mcomella, Assigned: mcomella)

References

Details

Attachments

(1 file)

See bug 962968 comment 0 for the motivation. bug 962968 knocked out a few of them, but there are still some remaining [1].

Note that SparseArray finds keys via binary search, rather than hashing, so there is also a cpu-based performance difference. rnewman recommends profiling to determine how SparseArrays perform comparatively, particularly in highly traversed code paths (see bug 962968 comment 12).

[1]: https://mxr.mozilla.org/mozilla-central/search?string=Map%3CInteger&find=\.java%24&findi=&filter=^[^\0]*%24&hitlimit=&tree=mozilla-central
via lint:

 ../../src/main/java/org/mozilla/gecko/AndroidGamepadManager.java:143: Use new SparseArray<Gamepad>(...) instead for better performance

 140     public static void startup() {
 141         ThreadUtils.assertOnUiThread();
 142         if (!sStarted) {
 143             sGamepads = new HashMap<Integer, Gamepad>();
 144             sPendingGamepads = new HashMap<Integer, List<KeyEvent>>();
 145             scanForGamepads();

../../src/main/java/org/mozilla/gecko/AndroidGamepadManager.java:144: Use new SparseArray<List<KeyEvent>>(...) instead for better performance

 141         ThreadUtils.assertOnUiThread();
 142         if (!sStarted) {
 143             sGamepads = new HashMap<Integer, Gamepad>();
 144             sPendingGamepads = new HashMap<Integer, List<KeyEvent>>();
 145             scanForGamepads();
 146             addDeviceListener();

../../src/main/java/org/mozilla/gecko/Tabs.java:53: Use new SparseArray<Tab>(...) instead for better performance

  50     private volatile Tab mSelectedTab;
  51 
  52     // All accesses to mTabs must be synchronized on the Tabs instance.
  53     private final HashMap<Integer, Tab> mTabs = new HashMap<Integer, Tab>();
  54 
  55     private AccountManager mAccountManager;

Priority: 4 / 10
Category: Performance
Severity: Warning
Explanation: HashMap can be replaced with SparseArray.
For maps where the keys are of type integer, it's typically more efficient to use the Android SparseArray API. This check identifies scenarios where you might want to consider using SparseArray instead of HashMap for better performance.

This is particularly useful when the value types are primitives like ints, where you can use SparseIntArray and avoid auto-boxing the values from int to Integer.

If you need to construct a HashMap because you need to call an API outside of your control which requires a Map, you can suppress this warning using for example the @SuppressLint annotation.
Blocks: android-lint
Summary: [Follow-up] Consider using SparseArray for HashMap using numeric keys (again) → [Linter: UseSparseArrays] Consider using SparseArray for HashMap using numeric keys (again)
Moved Tabs.java changes to bug 1175321.
Assignee: nobody → michael.l.comella
Summary: [Linter: UseSparseArrays] Consider using SparseArray for HashMap using numeric keys (again) → [Linter: UseSparseArrays] Consider using SparseArray for HashMap in AndroidGamepadManager.java
Bug 963873 - Replace HashMap with SparseArray in AndroidGamepadManager. r=nalexander

I don't expect to have many devices attached to a Fennec instance so it's
unlkely we'll run into the performance issues of SparseArray when
adding/removing.
Attachment #8623335 - Flags: review?(nalexander)
Comment on attachment 8623335 [details]
MozReview Request: Bug 963873 - Replace HashMap with SparseArray in AndroidGamepadManager. r=nalexander

https://reviewboard.mozilla.org/r/11487/#review9911

Ship It!
Attachment #8623335 - Flags: review?(nalexander) → review+
https://hg.mozilla.org/mozilla-central/rev/f79a2670fba4
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 41
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.