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

Commit cc103033 authored by Jooyung Han's avatar Jooyung Han
Browse files

Do not call getStagedApexInfos repeatedly 

IApexService.getStagedApexInfos(session) is a costly operation. The
current implementation calls the method more than necessary.

- StagingManager calls it to get the list of apex names for each
  observer.
- Each observer calls it to get the info for each staged apex name.

In this change,, StagingManager passes the result to all observers and
each observer just use the result.

Bug: 370712193
Test: StagingManagerTest
Test: StagedInstallInternalTest
Change-Id: Ica92dbad7e72ee3c853a0fc34069c89f47f072ee
parent 2b3c5dc2

services/core/java/com/android/server/BinaryTransparencyService.java

+1 −1
Original line number Diff line number Diff line
@@ -1523,7 +1523,7 @@ public class BinaryTransparencyService extends SystemService { 
                @Override

                public void onApexStaged(ApexStagedEvent event) throws RemoteException {

                    Slog.d(TAG, "A new APEX has been staged for update. There are currently "

                            + event.stagedApexModuleNames.length + " APEX(s) staged for update. "

                            + event.stagedApexInfos.length + " APEX(s) staged for update. "

                            + "Scheduling measurement...");

                    UpdateMeasurementsJobService.scheduleBinaryMeasurements(mContext,

                            BinaryTransparencyService.this);

services/core/java/com/android/server/pm/DexOptHelper.java

+3 −1
Original line number Diff line number Diff line
@@ -87,6 +87,7 @@ import java.io.File; 
import java.nio.file.Path;

import java.nio.file.Paths;

import java.util.ArrayList;

import java.util.Arrays;

import java.util.Collection;

import java.util.Collections;

import java.util.Comparator;
@@ -813,7 +814,8 @@ public final class DexOptHelper { 


        @Override

        public void onApexStaged(@NonNull ApexStagedEvent event) {

            mArtManager.onApexStaged(event.stagedApexModuleNames);

            mArtManager.onApexStaged(Arrays.stream(event.stagedApexInfos)

                    .map(info -> info.moduleName).toArray(String[]::new));

        }

    }

}

services/core/java/com/android/server/pm/PackageManagerNative.java

+3 −10
Original line number Diff line number Diff line
@@ -20,7 +20,6 @@ import static android.content.pm.PackageManager.CERT_INPUT_SHA256; 


import static com.android.server.pm.PackageManagerService.TAG;



import android.annotation.Nullable;

import android.content.pm.ApplicationInfo;

import android.content.pm.IPackageManagerNative;

import android.content.pm.IStagedApexObserver;
@@ -184,14 +183,8 @@ final class PackageManagerNative extends IPackageManagerNative.Stub { 
    }



    @Override

    public String[] getStagedApexModuleNames() {

        return mPm.mInstallerService.getStagingManager()

                .getStagedApexModuleNames().toArray(new String[0]);

    }



    @Override

    @Nullable

    public StagedApexInfo getStagedApexInfo(String moduleName) {

        return mPm.mInstallerService.getStagingManager().getStagedApexInfo(moduleName);

    public StagedApexInfo[] getStagedApexInfos() {

        return mPm.mInstallerService.getStagingManager().getStagedApexInfos().toArray(

                new StagedApexInfo[0]);

    }

}

services/core/java/com/android/server/pm/StagingManager.java

+19 −42
Original line number Diff line number Diff line
@@ -17,7 +17,6 @@ 
package com.android.server.pm;



import android.annotation.NonNull;

import android.annotation.Nullable;

import android.apex.ApexInfo;

import android.apex.ApexSessionInfo;

import android.apex.ApexSessionParams;
@@ -38,7 +37,6 @@ import android.os.SystemProperties; 
import android.os.Trace;

import android.os.UserHandle;

import android.text.TextUtils;

import android.util.ArrayMap;

import android.util.ArraySet;

import android.util.IntArray;

import android.util.Slog;
@@ -65,9 +63,9 @@ import java.io.File; 
import java.io.FileReader;

import java.io.FileWriter;

import java.util.ArrayList;

import java.util.Arrays;

import java.util.Collections;

import java.util.List;

import java.util.Map;

import java.util.Set;

import java.util.concurrent.CompletableFuture;

import java.util.concurrent.ExecutionException;
@@ -807,14 +805,13 @@ public class StagingManager { 
    }



    /**

     * Returns ApexInfo about APEX contained inside the session as a {@code Map<String, ApexInfo>},

     * where the key of the map is the module name of the ApexInfo.

     * Returns ApexInfo about APEX contained inside the session.

     *

     * Returns an empty map if there is any error.

     * Returns an empty list if there is any error.

     */

    @VisibleForTesting

    @NonNull

    Map<String, ApexInfo> getStagedApexInfos(@NonNull StagedSession session) {

    List<ApexInfo> getStagedApexInfos(@NonNull StagedSession session) {

        Preconditions.checkArgument(session != null, "Session is null");

        Preconditions.checkArgument(!session.hasParentSessionId(),

                session.sessionId() + " session has parent session");
@@ -824,7 +821,7 @@ public class StagingManager { 
        // Even if caller calls this method on ready session, the session could be abandoned

        // right after this method is called.

        if (!session.isSessionReady() || session.isDestroyed()) {

            return Collections.emptyMap();

            return Collections.emptyList();

        }



        ApexSessionParams params = new ApexSessionParams();
@@ -838,38 +835,17 @@ public class StagingManager { 
            }

        }

        params.childSessionIds = childSessionIds.toArray();



        ApexInfo[] infos = mApexManager.getStagedApexInfos(params);

        Map<String, ApexInfo> result = new ArrayMap<>();

        for (ApexInfo info : infos) {

            result.put(info.moduleName, info);

        }

        return result;

        return Arrays.asList(mApexManager.getStagedApexInfos(params));

    }



    /**

     * Returns apex module names of all packages that are staged ready

     */

    List<String> getStagedApexModuleNames() {

        List<String> result = new ArrayList<>();

        synchronized (mStagedSessions) {

            for (int i = 0; i < mStagedSessions.size(); i++) {

                final StagedSession session = mStagedSessions.valueAt(i);

                if (!session.isSessionReady() || session.isDestroyed()

                        || session.hasParentSessionId() || !session.containsApexSession()) {

                    continue;

                }

                result.addAll(getStagedApexInfos(session).keySet());

            }

        }

        return result;

    }



    /**

     * Returns ApexInfo of the {@code moduleInfo} provided if it is staged, otherwise returns null.

     * Returns ApexInfo list about APEXes contained inside all staged sessions.

     *

     * Returns an empty list if there is any error.

     */

    @Nullable

    StagedApexInfo getStagedApexInfo(String moduleName) {

    @NonNull

    List<StagedApexInfo> getStagedApexInfos() {

        List<StagedApexInfo> result = new ArrayList<>();

        synchronized (mStagedSessions) {

            for (int i = 0; i < mStagedSessions.size(); i++) {

                final StagedSession session = mStagedSessions.valueAt(i);
@@ -877,8 +853,7 @@ public class StagingManager { 
                        || session.hasParentSessionId() || !session.containsApexSession()) {

                    continue;

                }

                ApexInfo ai = getStagedApexInfos(session).get(moduleName);

                if (ai != null) {

                getStagedApexInfos(session).stream().map(ai -> {

                    StagedApexInfo info = new StagedApexInfo();

                    info.moduleName = ai.moduleName;

                    info.diskImagePath = ai.modulePath;
@@ -886,17 +861,19 @@ public class StagingManager { 
                    info.versionName = ai.versionName;

                    info.hasClassPathJars = ai.hasClassPathJars;

                    return info;

                }).forEach(result::add);

            }

        }

        }

        return null;

        return result;

    }



    private void notifyStagedApexObservers() {

        synchronized (mStagedApexObservers) {

            for (IStagedApexObserver observer : mStagedApexObservers) {

            List<StagedApexInfo> stagedApexInfos = getStagedApexInfos();

            ApexStagedEvent event = new ApexStagedEvent();

                event.stagedApexModuleNames = getStagedApexModuleNames().toArray(new String[0]);

            event.stagedApexInfos =

                    stagedApexInfos.toArray(new StagedApexInfo[stagedApexInfos.size()]);

            for (IStagedApexObserver observer : mStagedApexObservers) {

                try {

                    observer.onApexStaged(event);

                } catch (RemoteException re) {

services/tests/mockingservicestests/src/com/android/server/pm/StagingManagerTest.java

+20 −36
Original line number Diff line number Diff line
@@ -72,7 +72,6 @@ import java.io.File; 
import java.util.ArrayList;

import java.util.Arrays;

import java.util.List;

import java.util.Map;

import java.util.concurrent.CompletableFuture;

import java.util.function.Predicate;
@@ -486,7 +485,7 @@ public class StagingManagerTest { 
            FakeStagedSession session = new FakeStagedSession(239);

            session.setIsApex(true);

            // Call and verify

            Map<String, ApexInfo> result = mStagingManager.getStagedApexInfos(session);

            var result = mStagingManager.getStagedApexInfos(session);

            assertThat(result).isEmpty();

        }

        // Invalid session: destroyed
@@ -496,7 +495,7 @@ public class StagingManagerTest { 
            session.setIsApex(true);

            session.setDestroyed(true);

            // Call and verify

            Map<String, ApexInfo> result = mStagingManager.getStagedApexInfos(session);

            var result = mStagingManager.getStagedApexInfos(session);

            assertThat(result).isEmpty();

        }

    }
@@ -520,8 +519,8 @@ public class StagingManagerTest { 
        when(mApexManager.getStagedApexInfos(any())).thenReturn(fakeApexInfos);



        // Call and verify

        Map<String, ApexInfo> result = mStagingManager.getStagedApexInfos(validSession);

        assertThat(result).containsExactly(fakeApexInfos[0].moduleName, fakeApexInfos[0]);

        List<ApexInfo> result = mStagingManager.getStagedApexInfos(validSession);

        assertThat(result).containsExactly(fakeApexInfos[0]);



        ArgumentCaptor<ApexSessionParams> argumentCaptor =

                ArgumentCaptor.forClass(ApexSessionParams.class);
@@ -544,9 +543,8 @@ public class StagingManagerTest { 
        when(mApexManager.getStagedApexInfos(any())).thenReturn(fakeApexInfos);



        // Call and verify

        Map<String, ApexInfo> result = mStagingManager.getStagedApexInfos(parentSession);

        assertThat(result).containsExactly(fakeApexInfos[0].moduleName, fakeApexInfos[0],

                fakeApexInfos[1].moduleName, fakeApexInfos[1]);

        List<ApexInfo> result = mStagingManager.getStagedApexInfos(parentSession);

        assertThat(result).containsExactly(fakeApexInfos[0], fakeApexInfos[1]);



        ArgumentCaptor<ApexSessionParams> argumentCaptor =

                ArgumentCaptor.forClass(ApexSessionParams.class);
@@ -557,7 +555,7 @@ public class StagingManagerTest { 
    }



    @Test

    public void getStagedApexModuleNames_returnsStagedApexModules() throws Exception {

    public void getStagedApexInfos_returnsStagedApexModules() throws Exception {

        FakeStagedSession validSession1 = new FakeStagedSession(239);

        validSession1.setIsApex(true);

        validSession1.setSessionReady();
@@ -575,8 +573,8 @@ public class StagingManagerTest { 


        mockApexManagerGetStagedApexInfoWithSessionId();



        List<String> result = mStagingManager.getStagedApexModuleNames();

        assertThat(result).containsExactly("239", "123", "124");

        List<StagedApexInfo> result = mStagingManager.getStagedApexInfos();

        assertThat(result).containsExactly((Object[]) fakeStagedApexInfos("239", "123", "124"));

        verify(mApexManager, times(2)).getStagedApexInfos(any());

    }
@@ -605,26 +603,12 @@ public class StagingManagerTest { 
        });

    }



    @Test

    public void getStagedApexInfo() throws Exception {

        FakeStagedSession validSession1 = new FakeStagedSession(239);

        validSession1.setIsApex(true);

        validSession1.setSessionReady();

        mStagingManager.createSession(validSession1);

        ApexInfo[] fakeApexInfos = getFakeApexInfo(Arrays.asList("module1"));

        when(mApexManager.getStagedApexInfos(any())).thenReturn(fakeApexInfos);



        // Verify null is returned if module name is not found

        StagedApexInfo result = mStagingManager.getStagedApexInfo("not found");

        assertThat(result).isNull();

        verify(mApexManager, times(1)).getStagedApexInfos(any());

        // Otherwise, the correct object is returned

        result = mStagingManager.getStagedApexInfo("module1");

        assertThat(result.moduleName).isEqualTo(fakeApexInfos[0].moduleName);

        assertThat(result.diskImagePath).isEqualTo(fakeApexInfos[0].modulePath);

        assertThat(result.versionCode).isEqualTo(fakeApexInfos[0].versionCode);

        assertThat(result.versionName).isEqualTo(fakeApexInfos[0].versionName);

        verify(mApexManager, times(2)).getStagedApexInfos(any());

    private StagedApexInfo[] fakeStagedApexInfos(String... moduleNames) {

        return Arrays.stream(moduleNames).map(moduleName -> {

            StagedApexInfo info = new StagedApexInfo();

            info.moduleName = moduleName;

            return info;

        }).toArray(StagedApexInfo[]::new);

    }



    @Test
@@ -646,8 +630,8 @@ public class StagingManagerTest { 
            ArgumentCaptor<ApexStagedEvent> argumentCaptor = ArgumentCaptor.forClass(

                    ApexStagedEvent.class);

            verify(observer, times(1)).onApexStaged(argumentCaptor.capture());

            assertThat(argumentCaptor.getValue().stagedApexModuleNames).isEqualTo(

                    new String[]{"239"});

            assertThat(argumentCaptor.getValue().stagedApexInfos).isEqualTo(

                    fakeStagedApexInfos("239"));

        }



        // Create another staged session and verify observers are notified of union
@@ -662,8 +646,8 @@ public class StagingManagerTest { 
            ArgumentCaptor<ApexStagedEvent> argumentCaptor = ArgumentCaptor.forClass(

                    ApexStagedEvent.class);

            verify(observer, times(1)).onApexStaged(argumentCaptor.capture());

            assertThat(argumentCaptor.getValue().stagedApexModuleNames).isEqualTo(

                    new String[]{"239", "240"});

            assertThat(argumentCaptor.getValue().stagedApexInfos).isEqualTo(

                    fakeStagedApexInfos("239", "240"));

        }



        // Finally, verify that once unregistered, observer is not notified
@@ -699,7 +683,7 @@ public class StagingManagerTest { 
        ArgumentCaptor<ApexStagedEvent> argumentCaptor = ArgumentCaptor.forClass(

                ApexStagedEvent.class);

        verify(observer, times(1)).onApexStaged(argumentCaptor.capture());

        assertThat(argumentCaptor.getValue().stagedApexModuleNames).hasLength(0);

        assertThat(argumentCaptor.getValue().stagedApexInfos).hasLength(0);

    }



    @Test