From 67ba14b5b9514d62f4a61c47fa3c93be9b10e0ee Mon Sep 17 00:00:00 2001 From: Daniel Gultsch Date: Sun, 26 Jan 2025 14:01:27 +0100 Subject: [PATCH] reduce avatar cache invalidation --- .../siacs/conversations/entities/Contact.java | 11 ++-- .../conversations/entities/MucOptions.java | 19 +++---- .../conversations/parser/MessageParser.java | 22 +++++--- .../conversations/parser/PresenceParser.java | 54 ++++++++++--------- .../conversations/services/AvatarService.java | 54 ++++++++++--------- .../services/XmppConnectionService.java | 28 +++++----- .../conversations/ui/EnterNameActivity.java | 2 +- 7 files changed, 106 insertions(+), 84 deletions(-) diff --git a/src/main/java/eu/siacs/conversations/entities/Contact.java b/src/main/java/eu/siacs/conversations/entities/Contact.java index b15381e58c0e63824c999b6a2cb55c29094496c7..6508d00a966b88fccc8c66e61095c9c49be3c30f 100644 --- a/src/main/java/eu/siacs/conversations/entities/Contact.java +++ b/src/main/java/eu/siacs/conversations/entities/Contact.java @@ -448,21 +448,22 @@ public class Contact implements ListItem, Blockable { return getJid().getDomain().toString(); } - public void setAvatar(Avatar avatar) { - setAvatar(avatar, false); + public boolean setAvatar(final Avatar avatar) { + return setAvatar(avatar, false); } - public void setAvatar(Avatar avatar, boolean previouslyOmittedPepFetch) { + public boolean setAvatar(final Avatar avatar, final boolean previouslyOmittedPepFetch) { if (this.avatar != null && this.avatar.equals(avatar)) { - return; + return false; } if (!previouslyOmittedPepFetch && this.avatar != null && this.avatar.origin == Avatar.Origin.PEP && avatar.origin == Avatar.Origin.VCARD) { - return; + return false; } this.avatar = avatar; + return true; } public String getAvatarFilename() { diff --git a/src/main/java/eu/siacs/conversations/entities/MucOptions.java b/src/main/java/eu/siacs/conversations/entities/MucOptions.java index a1edc2a036d7970f5d5370b7dd6cefc107fb3e24..1b68acf0fceb5810d9e0df31ea333a80f242a9d0 100644 --- a/src/main/java/eu/siacs/conversations/entities/MucOptions.java +++ b/src/main/java/eu/siacs/conversations/entities/MucOptions.java @@ -60,7 +60,7 @@ public class MucOptions { return this.conversation.getAccount(); } - public boolean setSelf(User user) { + public boolean setSelf(final User user) { this.self = user; final boolean roleChanged = this.conversation.setAttribute("role", user.role.toString()); final boolean affiliationChanged = @@ -68,8 +68,8 @@ public class MucOptions { return roleChanged || affiliationChanged; } - public void changeAffiliation(Jid jid, Affiliation affiliation) { - User user = findUserByRealJid(jid); + public void changeAffiliation(final Jid jid, final Affiliation affiliation) { + final User user = findUserByRealJid(jid); synchronized (users) { if (user != null && user.getRole() == Role.NONE) { users.remove(user); @@ -440,14 +440,15 @@ public class MucOptions { } } - public List getUsers(int max) { - ArrayList subset = new ArrayList<>(); - HashSet jids = new HashSet<>(); - jids.add(account.getJid().asBareJid()); + public List getUsers(final int max) { + final ArrayList subset = new ArrayList<>(); + final HashSet addresses = new HashSet<>(); + addresses.add(account.getJid().asBareJid()); synchronized (users) { for (User user : users) { if (user.getRealJid() == null - || (user.getRealJid().getLocal() != null && jids.add(user.getRealJid()))) { + || (user.getRealJid().getLocal() != null + && addresses.add(user.getRealJid()))) { subset.add(user); } if (subset.size() >= max) { @@ -877,7 +878,7 @@ public class MucOptions { } } - public boolean setAvatar(Avatar avatar) { + public boolean setAvatar(final Avatar avatar) { if (this.avatar != null && this.avatar.equals(avatar)) { return false; } else { diff --git a/src/main/java/eu/siacs/conversations/parser/MessageParser.java b/src/main/java/eu/siacs/conversations/parser/MessageParser.java index 9188c1b661ab828341f3fb9ec6363c44f517e581..226c7511474ee03a93bda323d8e9c9f9deed953a 100644 --- a/src/main/java/eu/siacs/conversations/parser/MessageParser.java +++ b/src/main/java/eu/siacs/conversations/parser/MessageParser.java @@ -266,11 +266,12 @@ public class MessageParser extends AbstractParser mXmppConnectionService.updateAccountUi(); } else { final Contact contact = account.getRoster().getContact(from); - contact.setAvatar(avatar); - mXmppConnectionService.syncRoster(account); - mXmppConnectionService.getAvatarService().clear(contact); - mXmppConnectionService.updateConversationUi(); - mXmppConnectionService.updateRosterUi(); + if (contact.setAvatar(avatar)) { + mXmppConnectionService.syncRoster(account); + mXmppConnectionService.getAvatarService().clear(contact); + mXmppConnectionService.updateConversationUi(); + mXmppConnectionService.updateRosterUi(); + } } } else if (mXmppConnectionService.isDataSaverDisabled()) { mXmppConnectionService.fetchAvatar(account, avatar); @@ -1174,7 +1175,7 @@ public class MessageParser extends AbstractParser // ignored } } else if ("item".equals(child.getName())) { - MucOptions.User user = AbstractParser.parseItem(conversation, child); + final var user = AbstractParser.parseItem(conversation, child); Log.d( Config.LOGTAG, account.getJid() @@ -1185,8 +1186,13 @@ public class MessageParser extends AbstractParser + " in " + conversation.getJid().asBareJid()); if (!user.realJidMatchesAccount()) { - boolean isNew = conversation.getMucOptions().updateUser(user); - mXmppConnectionService.getAvatarService().clear(conversation); + final var mucOptions = conversation.getMucOptions(); + final boolean isNew = mucOptions.updateUser(user); + final var avatarService = mXmppConnectionService.getAvatarService(); + if (Strings.isNullOrEmpty(mucOptions.getAvatar())) { + avatarService.clear(mucOptions); + } + avatarService.clear(user); mXmppConnectionService.updateMucRosterUi(); mXmppConnectionService.updateConversationUi(); Contact contact = user.getContact(); diff --git a/src/main/java/eu/siacs/conversations/parser/PresenceParser.java b/src/main/java/eu/siacs/conversations/parser/PresenceParser.java index 088938402ec24c4e6958dbd5660bfcddbf9a8fec..eef615de358c79ba745f5695913b476d8def213a 100644 --- a/src/main/java/eu/siacs/conversations/parser/PresenceParser.java +++ b/src/main/java/eu/siacs/conversations/parser/PresenceParser.java @@ -1,6 +1,7 @@ package eu.siacs.conversations.parser; import android.util.Log; +import com.google.common.base.Strings; import eu.siacs.conversations.Config; import eu.siacs.conversations.crypto.PgpEngine; import eu.siacs.conversations.crypto.axolotl.AxolotlService; @@ -37,22 +38,24 @@ public class PresenceParser extends AbstractParser packet.getFrom() == null ? null : mXmppConnectionService.find(account, packet.getFrom().asBareJid()); - if (conversation != null) { - final MucOptions mucOptions = conversation.getMucOptions(); - boolean before = mucOptions.online(); - int count = mucOptions.getUserCount(); - final List tileUserBefore = mucOptions.getUsers(5); - processConferencePresence(packet, conversation); - final List tileUserAfter = mucOptions.getUsers(5); - if (!tileUserAfter.equals(tileUserBefore)) { - mXmppConnectionService.getAvatarService().clear(mucOptions); - } - if (before != mucOptions.online() - || (mucOptions.online() && count != mucOptions.getUserCount())) { - mXmppConnectionService.updateConversationUi(); - } else if (mucOptions.online()) { - mXmppConnectionService.updateMucRosterUi(); - } + if (conversation == null) { + return; + } + final MucOptions mucOptions = conversation.getMucOptions(); + boolean before = mucOptions.online(); + int count = mucOptions.getUserCount(); + final List tileUserBefore = mucOptions.getUsers(5); + processConferencePresence(packet, conversation); + final List tileUserAfter = mucOptions.getUsers(5); + if (Strings.isNullOrEmpty(mucOptions.getAvatar()) + && !tileUserAfter.equals(tileUserBefore)) { + mXmppConnectionService.getAvatarService().clear(mucOptions); + } + if (before != mucOptions.online() + || (mucOptions.online() && count != mucOptions.getUserCount())) { + mXmppConnectionService.updateConversationUi(); + } else if (mucOptions.online()) { + mXmppConnectionService.updateMucRosterUi(); } } @@ -152,9 +155,11 @@ public class PresenceParser extends AbstractParser .getAccount() .getRoster() .getContact(user.getRealJid()); - c.setAvatar(avatar); - mXmppConnectionService.syncRoster(conversation.getAccount()); - mXmppConnectionService.getAvatarService().clear(c); + if (c.setAvatar(avatar)) { + mXmppConnectionService.syncRoster( + conversation.getAccount()); + mXmppConnectionService.getAvatarService().clear(c); + } mXmppConnectionService.updateRosterUi(); } } else if (mXmppConnectionService.isDataSaverDisabled()) { @@ -329,11 +334,12 @@ public class PresenceParser extends AbstractParser mXmppConnectionService.updateConversationUi(); mXmppConnectionService.updateAccountUi(); } else { - contact.setAvatar(avatar); - mXmppConnectionService.syncRoster(account); - mXmppConnectionService.getAvatarService().clear(contact); - mXmppConnectionService.updateConversationUi(); - mXmppConnectionService.updateRosterUi(); + if (contact.setAvatar(avatar)) { + mXmppConnectionService.syncRoster(account); + mXmppConnectionService.getAvatarService().clear(contact); + mXmppConnectionService.updateConversationUi(); + mXmppConnectionService.updateRosterUi(); + } } } else if (mXmppConnectionService.isDataSaverDisabled()) { mXmppConnectionService.fetchAvatar(account, avatar); diff --git a/src/main/java/eu/siacs/conversations/services/AvatarService.java b/src/main/java/eu/siacs/conversations/services/AvatarService.java index 46021557624fff321a0cda32724cb2f7072ce701..4f2bdc2d69ae128f2db524f28ceb61c60cb65d79 100644 --- a/src/main/java/eu/siacs/conversations/services/AvatarService.java +++ b/src/main/java/eu/siacs/conversations/services/AvatarService.java @@ -15,10 +15,10 @@ import android.net.Uri; import android.text.TextUtils; import android.util.DisplayMetrics; import android.util.Log; -import android.util.LruCache; import androidx.annotation.ColorInt; import androidx.annotation.Nullable; import androidx.core.content.res.ResourcesCompat; +import com.google.common.base.Strings; import eu.siacs.conversations.Config; import eu.siacs.conversations.R; import eu.siacs.conversations.entities.Account; @@ -57,6 +57,7 @@ public class AvatarService implements OnAdvancedStreamFeaturesLoaded { private static final String CHANNEL_SYMBOL = "#"; private final Set sizes = new HashSet<>(); + // TODO refactor to multimap private final HashMap> conversationDependentKeys = new HashMap<>(); protected XmppConnectionService mXmppConnectionService = null; @@ -261,23 +262,27 @@ public class AvatarService implements OnAdvancedStreamFeaturesLoaded { return avatar; } - public void clear(Contact contact) { + public void clear(final Contact contact) { synchronized (this.sizes) { for (final Integer size : sizes) { this.mXmppConnectionService.getBitmapCache().remove(key(contact, size)); } } - for (Conversation conversation : mXmppConnectionService.findAllConferencesWith(contact)) { - MucOptions.User user = - conversation.getMucOptions().findUserByRealJid(contact.getJid().asBareJid()); + for (final Conversation conversation : + mXmppConnectionService.findAllConferencesWith(contact)) { + final var mucOptions = conversation.getMucOptions(); + final var user = mucOptions.findUserByRealJid(contact.getJid().asBareJid()); if (user != null) { clear(user); } - clear(conversation); + if (Strings.isNullOrEmpty(mucOptions.getAvatar()) + && mucOptions.isPrivateAndNonAnonymous()) { + clear(mucOptions); + } } } - private String key(Contact contact, int size) { + private String key(final Contact contact, final int size) { synchronized (this.sizes) { this.sizes.add(size); } @@ -345,26 +350,15 @@ public class AvatarService implements OnAdvancedStreamFeaturesLoaded { } } - public void clear(Conversation conversation) { + public void clear(final Conversation conversation) { if (conversation.getMode() == Conversation.MODE_SINGLE) { clear(conversation.getContact()); } else { clear(conversation.getMucOptions()); - synchronized (this.conversationDependentKeys) { - Set keys = this.conversationDependentKeys.get(conversation.getUuid()); - if (keys == null) { - return; - } - LruCache cache = this.mXmppConnectionService.getBitmapCache(); - for (String key : keys) { - cache.remove(key); - } - keys.clear(); - } } } - private Bitmap get(MucOptions mucOptions, int size, boolean cachedOnly) { + private Bitmap get(final MucOptions mucOptions, final int size, final boolean cachedOnly) { final String KEY = key(mucOptions, size); Bitmap bitmap = this.mXmppConnectionService.getBitmapCache().get(KEY); if (bitmap != null || cachedOnly) { @@ -377,7 +371,7 @@ public class AvatarService implements OnAdvancedStreamFeaturesLoaded { Conversation c = mucOptions.getConversation(); if (mucOptions.isPrivateAndNonAnonymous()) { final List users = mucOptions.getUsersRelevantForNameAndAvatar(); - if (users.size() == 0) { + if (users.isEmpty()) { bitmap = getImpl( c.getName().toString(), @@ -456,12 +450,12 @@ public class AvatarService implements OnAdvancedStreamFeaturesLoaded { return PREFIX_CONVERSATION + "_" + options.getConversation().getUuid() + "_" + size; } - private String key(List users, int size) { + private String key(final List users, final int size) { final Conversation conversation = users.get(0).getConversation(); StringBuilder builder = new StringBuilder("TILE_"); builder.append(conversation.getUuid()); - for (MucOptions.User user : users) { + for (final MucOptions.User user : users) { builder.append("\0"); builder.append(emptyOnNull(user.getRealJid())); builder.append("\0"); @@ -551,12 +545,24 @@ public class AvatarService implements OnAdvancedStreamFeaturesLoaded { } } - public void clear(MucOptions.User user) { + public void clear(final MucOptions.User user) { synchronized (this.sizes) { for (Integer size : sizes) { this.mXmppConnectionService.getBitmapCache().remove(key(user, size)); } } + synchronized (this.conversationDependentKeys) { + final Set keys = + this.conversationDependentKeys.get(user.getConversation().getUuid()); + if (keys == null) { + return; + } + final var cache = this.mXmppConnectionService.getBitmapCache(); + for (final String key : keys) { + cache.remove(key); + } + keys.clear(); + } } private String key(Account account, int size) { diff --git a/src/main/java/eu/siacs/conversations/services/XmppConnectionService.java b/src/main/java/eu/siacs/conversations/services/XmppConnectionService.java index 9a5ea426f7bd80f1126ff2f204878eb5ceff15b5..0ff7dbe6fceb11fd24763ee7b8cfb8b3785da4e2 100644 --- a/src/main/java/eu/siacs/conversations/services/XmppConnectionService.java +++ b/src/main/java/eu/siacs/conversations/services/XmppConnectionService.java @@ -3455,15 +3455,15 @@ public class XmppConnectionService extends Service { } public boolean checkListeners() { - return (this.mOnAccountUpdates.size() == 0 - && this.mOnConversationUpdates.size() == 0 - && this.mOnRosterUpdates.size() == 0 - && this.mOnCaptchaRequested.size() == 0 - && this.mOnMucRosterUpdate.size() == 0 - && this.mOnUpdateBlocklist.size() == 0 - && this.mOnShowErrorToasts.size() == 0 - && this.onJingleRtpConnectionUpdate.size() == 0 - && this.mOnKeyStatusUpdated.size() == 0); + return (this.mOnAccountUpdates.isEmpty() + && this.mOnConversationUpdates.isEmpty() + && this.mOnRosterUpdates.isEmpty() + && this.mOnCaptchaRequested.isEmpty() + && this.mOnMucRosterUpdate.isEmpty() + && this.mOnUpdateBlocklist.isEmpty() + && this.mOnShowErrorToasts.isEmpty() + && this.onJingleRtpConnectionUpdate.isEmpty() + && this.mOnKeyStatusUpdated.isEmpty()); } private void switchToForeground() { @@ -3815,7 +3815,8 @@ public class XmppConnectionService extends Service { } ++i; if (i >= affiliations.length) { - List members = conversation.getMucOptions().getMembers(true); + final var mucOptions = conversation.getMucOptions(); + final var members = mucOptions.getMembers(true); if (success) { List cryptoTargets = conversation.getAcceptedCryptoTargets(); boolean changed = false; @@ -3840,7 +3841,7 @@ public class XmppConnectionService extends Service { updateConversation(conversation); } } - getAvatarService().clear(conversation); + getAvatarService().clear(mucOptions); updateMucRosterUi(); updateConversationUi(); } @@ -4487,8 +4488,9 @@ public class XmppConnectionService extends Service { request, (response) -> { if (response.getType() == Iq.Type.RESULT) { - conference.getMucOptions().changeAffiliation(jid, affiliation); - getAvatarService().clear(conference); + final var mucOptions = conference.getMucOptions(); + mucOptions.changeAffiliation(jid, affiliation); + getAvatarService().clear(mucOptions); if (callback != null) { callback.onAffiliationChangedSuccessful(jid); } else { diff --git a/src/quicksy/java/eu/siacs/conversations/ui/EnterNameActivity.java b/src/quicksy/java/eu/siacs/conversations/ui/EnterNameActivity.java index 38d1f0db49d1e4dbf87a573244262d4a5b6ea3da..42c2d3c6fc453e4b1bab68ec011ab9f432728210 100644 --- a/src/quicksy/java/eu/siacs/conversations/ui/EnterNameActivity.java +++ b/src/quicksy/java/eu/siacs/conversations/ui/EnterNameActivity.java @@ -54,7 +54,7 @@ public class EnterNameActivity extends XmppActivity } @Override - public void onSaveInstanceState(Bundle savedInstanceState) { + public void onSaveInstanceState(final Bundle savedInstanceState) { savedInstanceState.putBoolean("set_nick", this.setNick.get()); super.onSaveInstanceState(savedInstanceState); }