From 67fed01522ad26c850b20d2c960e4d392dc401d1 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Tue, 17 Aug 2010 10:29:15 -0700 Subject: [PATCH] Avoid unnecessary SharedPrefences disk writes. Apps commonly edit + commit redundant changes to their SharedPreferences, not checking the existing values. Rather than force all apps to double-check that their settings writes aren't redundant, we should just make .commit() faster (avoiding the disk write) when the file already exists on disk and no effective changes were made. Change-Id: I7edbd0d3ace5b69b7af6d12c39797c8b7f86230b --- core/java/android/app/ContextImpl.java | 34 +++++++++++++++++++++----- 1 file changed, 28 insertions(+), 6 deletions(-) diff --git a/core/java/android/app/ContextImpl.java b/core/java/android/app/ContextImpl.java index e99d4b4dbbd53..2870c50b8658d 100644 --- a/core/java/android/app/ContextImpl.java +++ b/core/java/android/app/ContextImpl.java @@ -2844,6 +2844,7 @@ class ContextImpl extends Context { boolean returnValue; boolean hasListeners; + boolean changesMade = false; List keysModified = null; Set listeners = null; @@ -2857,17 +2858,31 @@ class ContextImpl extends Context { synchronized (this) { if (mClear) { - mMap.clear(); + if (!mMap.isEmpty()) { + changesMade = true; + mMap.clear(); + } mClear = false; } for (Entry e : mModified.entrySet()) { String k = e.getKey(); Object v = e.getValue(); - if (v == this) { - mMap.remove(k); + if (v == this) { // magic value for a removal mutation + if (mMap.containsKey(k)) { + mMap.remove(k); + changesMade = true; + } } else { - mMap.put(k, v); + boolean isSame = false; + if (mMap.containsKey(k)) { + Object existingValue = mMap.get(k); + isSame = existingValue != null && existingValue.equals(v); + } + if (!isSame) { + mMap.put(k, v); + changesMade = true; + } } if (hasListeners) { @@ -2878,7 +2893,7 @@ class ContextImpl extends Context { mModified.clear(); } - returnValue = writeFileLocked(); + returnValue = writeFileLocked(changesMade); } if (hasListeners) { @@ -2923,9 +2938,16 @@ class ContextImpl extends Context { return str; } - private boolean writeFileLocked() { + private boolean writeFileLocked(boolean changesMade) { // Rename the current file so it may be used as a backup during the next read if (mFile.exists()) { + if (!changesMade) { + // If the file already exists, but no changes were + // made to the underlying map, it's wasteful to + // re-write the file. Return as if we wrote it + // out. + return true; + } if (!mBackupFile.exists()) { if (!mFile.renameTo(mBackupFile)) { Log.e(TAG, "Couldn't rename file " + mFile