Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit b562e7e7 authored by Neil Fuller's avatar Neil Fuller
Browse files

Tidy ups from prior code reviews 

Switch to new(ish) Set.of() for construction of an immutable Set from an
array of elements. Update ServerFlags to be more explicit about lookup
behavior.

Bug: 197624972
Test: Build only
Change-Id: Ib86ffabf6b2ab47d89bda2cadb345eb9fdd42dbf
parent 154e4da6

services/core/java/com/android/server/timedetector/ServerFlags.java

+27 −7
Original line number Diff line number Diff line
@@ -35,6 +35,7 @@ import java.lang.annotation.Target; 
import java.time.DateTimeException;

import java.time.Duration;

import java.time.Instant;

import java.util.HashSet;

import java.util.Map;

import java.util.Objects;

import java.util.Optional;
@@ -179,8 +180,13 @@ public final class ServerFlags { 
    public static final @DeviceConfigKey String KEY_ENHANCED_METRICS_COLLECTION_ENABLED =

            "enhanced_metrics_collection_enabled";



    /**

     * The registered listeners and the keys to trigger on. The value is explicitly a HashSet to

     * ensure O(1) lookup performance when working out whether a listener should trigger.

     */

    @GuardedBy("mListeners")

    private final ArrayMap<ConfigurationChangeListener, Set<String>> mListeners = new ArrayMap<>();

    private final ArrayMap<ConfigurationChangeListener, HashSet<String>> mListeners =

            new ArrayMap<>();



    private static final Object SLOCK = new Object();
@@ -207,18 +213,29 @@ public final class ServerFlags { 


    private void handlePropertiesChanged(@NonNull DeviceConfig.Properties properties) {

        synchronized (mListeners) {

            for (Map.Entry<ConfigurationChangeListener, Set<String>> listenerEntry

            for (Map.Entry<ConfigurationChangeListener, HashSet<String>> listenerEntry

                    : mListeners.entrySet()) {

                if (intersects(listenerEntry.getValue(), properties.getKeyset())) {

                // It's unclear which set of the following two Sets is going to be larger in the

                // average case: monitoredKeys will be a subset of the set of possible keys, but

                // only changed keys are reported. Because we guarantee the type / lookup behavior

                // of the monitoredKeys by making that a HashSet, that is used as the haystack Set,

                // while the changed keys is treated as the needles Iterable. At the time of

                // writing, properties.getKeyset() actually returns a HashSet, so iteration isn't

                // super efficient and the use of HashSet for monitoredKeys may be redundant, but

                // neither set will be enormous.

                HashSet<String> monitoredKeys = listenerEntry.getValue();

                Iterable<String> modifiedKeys = properties.getKeyset();

                if (containsAny(monitoredKeys, modifiedKeys)) {

                    listenerEntry.getKey().onChange();

                }

            }

        }

    }



    private static boolean intersects(@NonNull Set<String> one, @NonNull Set<String> two) {

        for (String toFind : one) {

            if (two.contains(toFind)) {

    private static boolean containsAny(

            @NonNull Set<String> haystack, @NonNull Iterable<String> needles) {

        for (String needle : needles) {

            if (haystack.contains(needle)) {

                return true;

            }

        }
@@ -237,8 +254,11 @@ public final class ServerFlags { 
        Objects.requireNonNull(listener);

        Objects.requireNonNull(keys);



        // Make a defensive copy and use a well-defined Set implementation to provide predictable

        // performance on the lookup.

        HashSet<String> keysCopy = new HashSet<>(keys);

        synchronized (mListeners) {

            mListeners.put(listener, keys);

            mListeners.put(listener, keysCopy);

        }

    }

services/core/java/com/android/server/timedetector/ServiceConfigAccessor.java

+4 −7
Original line number Diff line number Diff line
@@ -25,7 +25,6 @@ import android.annotation.Nullable; 
import android.content.Context;

import android.os.Build;

import android.os.SystemProperties;

import android.util.ArraySet;



import com.android.internal.R;

import com.android.internal.annotations.GuardedBy;
@@ -35,7 +34,6 @@ import com.android.server.timezonedetector.ConfigurationChangeListener; 


import java.time.Instant;

import java.util.Arrays;

import java.util.Collections;

import java.util.Objects;

import java.util.Optional;

import java.util.Set;
@@ -65,11 +63,10 @@ final class ServiceConfigAccessor { 
            Long.max(android.os.Environment.getRootDirectory().lastModified(), Build.TIME));



    /** Device config keys that affect the {@link TimeDetectorService}. */

    private static final Set<String> SERVER_FLAGS_KEYS_TO_WATCH = Collections.unmodifiableSet(

            new ArraySet<>(new String[] {

    private static final Set<String> SERVER_FLAGS_KEYS_TO_WATCH = Set.of(

            KEY_TIME_DETECTOR_LOWER_BOUND_MILLIS_OVERRIDE,

                    KEY_TIME_DETECTOR_ORIGIN_PRIORITIES_OVERRIDE,

            }));

            KEY_TIME_DETECTOR_ORIGIN_PRIORITIES_OVERRIDE

    );



    private static final Object SLOCK = new Object();

services/core/java/com/android/server/timezonedetector/ServiceConfigAccessorImpl.java

+20 −24
Original line number Diff line number Diff line
@@ -36,7 +36,6 @@ import android.location.LocationManager; 
import android.os.UserHandle;

import android.os.UserManager;

import android.provider.Settings;

import android.util.ArraySet;



import com.android.internal.R;

import com.android.internal.annotations.GuardedBy;
@@ -45,7 +44,6 @@ import com.android.server.timedetector.ServerFlags; 


import java.time.Duration;

import java.util.ArrayList;

import java.util.Collections;

import java.util.List;

import java.util.Objects;

import java.util.Optional;
@@ -59,22 +57,20 @@ public final class ServiceConfigAccessorImpl implements ServiceConfigAccessor { 
    /**

     * Device config keys that can affect the content of {@link ConfigurationInternal}.

     */

    private static final Set<String> CONFIGURATION_INTERNAL_SERVER_FLAGS_KEYS_TO_WATCH =

            Collections.unmodifiableSet(new ArraySet<>(new String[] {

    private static final Set<String> CONFIGURATION_INTERNAL_SERVER_FLAGS_KEYS_TO_WATCH = Set.of(

            ServerFlags.KEY_LOCATION_TIME_ZONE_DETECTION_FEATURE_SUPPORTED,

            ServerFlags.KEY_LOCATION_TIME_ZONE_DETECTION_RUN_IN_BACKGROUND_ENABLED,

            ServerFlags.KEY_ENHANCED_METRICS_COLLECTION_ENABLED,

            ServerFlags.KEY_LOCATION_TIME_ZONE_DETECTION_SETTING_ENABLED_DEFAULT,

            ServerFlags.KEY_LOCATION_TIME_ZONE_DETECTION_SETTING_ENABLED_OVERRIDE,

                    ServerFlags.KEY_TIME_ZONE_DETECTOR_TELEPHONY_FALLBACK_SUPPORTED,

            }));

            ServerFlags.KEY_TIME_ZONE_DETECTOR_TELEPHONY_FALLBACK_SUPPORTED

    );



    /**

     * Device config keys that can affect {@link

     * com.android.server.timezonedetector.location.LocationTimeZoneManagerService} behavior.

     */

    private static final Set<String> LOCATION_TIME_ZONE_MANAGER_SERVER_FLAGS_KEYS_TO_WATCH =

            Collections.unmodifiableSet(new ArraySet<>(new String[] {

    private static final Set<String> LOCATION_TIME_ZONE_MANAGER_SERVER_FLAGS_KEYS_TO_WATCH = Set.of(

            ServerFlags.KEY_LOCATION_TIME_ZONE_DETECTION_FEATURE_SUPPORTED,

            ServerFlags.KEY_LOCATION_TIME_ZONE_DETECTION_RUN_IN_BACKGROUND_ENABLED,

            ServerFlags.KEY_LOCATION_TIME_ZONE_DETECTION_SETTING_ENABLED_DEFAULT,
@@ -85,7 +81,7 @@ public final class ServiceConfigAccessorImpl implements ServiceConfigAccessor { 
            ServerFlags.KEY_LTZP_INITIALIZATION_TIMEOUT_FUZZ_MILLIS,

            ServerFlags.KEY_LTZP_EVENT_FILTERING_AGE_THRESHOLD_MILLIS,

            ServerFlags.KEY_LOCATION_TIME_ZONE_DETECTION_UNCERTAINTY_DELAY_MILLIS

            }));

    );



    private static final Duration DEFAULT_LTZP_INITIALIZATION_TIMEOUT = Duration.ofMinutes(5);

    private static final Duration DEFAULT_LTZP_INITIALIZATION_TIMEOUT_FUZZ = Duration.ofMinutes(1);