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

Commit 0827e1d0 authored by Tim Van Patten's avatar Tim Van Patten
Browse files

FeatureOverrides: Remove copy constructor

Remove the FeatureOverrides copy constructor to reduces the amount of
bytes copied on each app launch to just the Binder call.

While debugging b/425292768, it stood out that the FeatureOverrides copy
constructor is unnecessary and inefficient.

When an app launches, there are two instances of FeatureOverrides:

1. gpuservice (the cached instance, parsed from the protobuf file)
2. GraphicsEnv (the per-app instance used by the loader to toggle
   features for each app).

GraphicsEnv is instantiated per-app, so it's instance of
FeatureOverrides needs to be populated by the cached values from
gpuservice. This is achieved with a Binder call from GraphicsEnv to
gpuservice. Before this CL, there were several FeatureOverrides copy
constructor calls, which is inefficient and unnecessary.

Instead, GraphicsEnv is updated to pass a reference to
BpGpuService::getFeatureOverrides(), so it's instance can be filled in
directly with the deserialized Binder data. Additionally,
FeatureOverrideParser returns a const reference to its cached
FeatureOverrides values to BnGpuService
eliminating the copy constructor. This leaves the Binder
serialize-deserialize as the only copying of the FeatureOverrides data.

Bug: b/425292768
Test: atest gpuservice_unittest
Flag: EXEMPT bugfix
Change-Id: I9cbacad24ac5601e8fa550517ee1913bf3000c73
parent 7598f6bf
Loading
Loading
Loading
Loading
+3 −0
Original line number Diff line number Diff line
@@ -138,6 +138,9 @@ status_t FeatureOverrides::writeToParcel(Parcel* parcel) const {
status_t FeatureOverrides::readFromParcel(const Parcel* parcel) {
    status_t status;

    mGlobalFeatures.clear();
    mPackageFeatures.clear();

    // Number of global feature configs.
    status = parcel->resizeOutVector(&mGlobalFeatures);
    if (status != OK) {
+1 −1
Original line number Diff line number Diff line
@@ -647,7 +647,7 @@ void GraphicsEnv::updateAngleFeatureOverrides() {
        return;
    }

    mFeatureOverrides = gpuService->getFeatureOverrides();
    gpuService->getFeatureOverrides(mFeatureOverrides);
}

void GraphicsEnv::getAngleFeatureOverrides(std::vector<const char*>& enabled,
+8 −6
Original line number Diff line number Diff line
@@ -132,19 +132,21 @@ public:
        return driverPath;
    }

    FeatureOverrides getFeatureOverrides() override {
    void getFeatureOverrides(FeatureOverrides& featureOverrides) override {
        Parcel data, reply;
        data.writeInterfaceToken(IGpuService::getInterfaceDescriptor());

        FeatureOverrides featureOverrides;
        status_t error =
                remote()->transact(BnGpuService::GET_FEATURE_CONFIG_OVERRIDES, data, &reply);
        if (error != OK) {
            return featureOverrides;
            return;
        }

        featureOverrides.readFromParcel(&reply);
        return featureOverrides;
        error = featureOverrides.readFromParcel(&reply);
        if (error != OK) {
            ALOGE("Failed to read FeatureOverrides from parcel: error = %d", error);
            return;
        }
    }
};

@@ -309,7 +311,7 @@ status_t BnGpuService::onTransact(uint32_t code, const Parcel& data, Parcel* rep

            // Get the FeatureOverrides from gpuservice, which implements the IGpuService interface
            // with GpuService::getFeatureOverrides().
            FeatureOverrides featureOverrides = getFeatureOverrides();
            const FeatureOverrides& featureOverrides = getCachedFeatureOverrides();
            featureOverrides.writeToParcel(reply);
            return OK;
        }
+5 −1
Original line number Diff line number Diff line
@@ -45,7 +45,7 @@ public:
class FeatureOverrides : public Parcelable {
public:
    FeatureOverrides() = default;
    FeatureOverrides(const FeatureOverrides&) = default;
    FeatureOverrides(const FeatureOverrides&) = delete;
    virtual ~FeatureOverrides() = default;
    virtual status_t writeToParcel(Parcel* parcel) const;
    virtual status_t readFromParcel(const Parcel* parcel);
@@ -56,4 +56,8 @@ public:
    std::map<std::string, std::vector<FeatureConfig>> mPackageFeatures;
};

// Assert that FeatureOverrides is NOT copy-constructible
static_assert(!std::is_copy_constructible<FeatureOverrides>::value,
              "FeatureOverrides should not be copy-constructible!");

} // namespace android
+2 −1
Original line number Diff line number Diff line
@@ -61,7 +61,7 @@ public:
    virtual std::string getPersistGraphicsEgl() = 0;

    // Get the list of features to override.
    virtual FeatureOverrides getFeatureOverrides() = 0;
    virtual void getFeatureOverrides(FeatureOverrides& featureOverrides) = 0;
};

class BnGpuService : public BnInterface<IGpuService> {
@@ -84,6 +84,7 @@ public:

protected:
    virtual status_t shellCommand(int in, int out, int err, std::vector<String16>& args) = 0;
    virtual const FeatureOverrides& getCachedFeatureOverrides() = 0;
};

} // namespace android
Loading