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

Commit 99b48bdd authored by ESWAR MAGATAPALLI (xWF)'s avatar ESWAR MAGATAPALLI (xWF)
Browse files

Revert "[res] Optimize isUpToDate() for ApkAssets"

Revert submission 31021037

Reason for revert: Droidmonitor created revert due to b/388342212. Will be verifying through ABTD before submission.

Reverted changes: /q/submissionid:31021037

Change-Id: I7d0eeaf7476c66f5276c68320f93f2b56953d531
parent 6e427525
Loading
Loading
Loading
Loading
+7 −49
Original line number Diff line number Diff line
@@ -25,7 +25,6 @@ import android.content.res.loader.ResourcesProvider;
import android.ravenwood.annotation.RavenwoodClassLoadHook;
import android.ravenwood.annotation.RavenwoodKeepWholeClass;
import android.text.TextUtils;
import android.util.Log;

import com.android.internal.annotations.GuardedBy;

@@ -51,7 +50,6 @@ import java.util.Objects;
@RavenwoodKeepWholeClass
@RavenwoodClassLoadHook(RavenwoodClassLoadHook.LIBANDROID_LOADING_HOOK)
public final class ApkAssets {
    private static final boolean DEBUG = false;

    /**
     * The apk assets contains framework resource values specified by the system.
@@ -136,17 +134,6 @@ public final class ApkAssets {
    @Nullable
    private final AssetsProvider mAssets;

    @NonNull
    private String mName;

    private static final int UPTODATE_FALSE = 0;
    private static final int UPTODATE_TRUE = 1;
    private static final int UPTODATE_ALWAYS_TRUE = 2;

    // Start with the only value that may change later and would force a native call to
    // double check it.
    private int mPreviousUpToDateResult = UPTODATE_TRUE;

    /**
     * Creates a new ApkAssets instance from the given path on disk.
     *
@@ -317,7 +304,7 @@ public final class ApkAssets {

    private ApkAssets(@FormatType int format, @NonNull String path, @PropertyFlags int flags,
            @Nullable AssetsProvider assets) throws IOException {
        this(format, flags, assets, path);
        this(format, flags, assets);
        Objects.requireNonNull(path, "path");
        mNativePtr = nativeLoad(format, path, flags, assets);
        mStringBlock = new StringBlock(nativeGetStringBlock(mNativePtr), true /*useSparse*/);
@@ -326,7 +313,7 @@ public final class ApkAssets {
    private ApkAssets(@FormatType int format, @NonNull FileDescriptor fd,
            @NonNull String friendlyName, @PropertyFlags int flags, @Nullable AssetsProvider assets)
            throws IOException {
        this(format, flags, assets, friendlyName);
        this(format, flags, assets);
        Objects.requireNonNull(fd, "fd");
        Objects.requireNonNull(friendlyName, "friendlyName");
        mNativePtr = nativeLoadFd(format, fd, friendlyName, flags, assets);
@@ -336,7 +323,7 @@ public final class ApkAssets {
    private ApkAssets(@FormatType int format, @NonNull FileDescriptor fd,
            @NonNull String friendlyName, long offset, long length, @PropertyFlags int flags,
            @Nullable AssetsProvider assets) throws IOException {
        this(format, flags, assets, friendlyName);
        this(format, flags, assets);
        Objects.requireNonNull(fd, "fd");
        Objects.requireNonNull(friendlyName, "friendlyName");
        mNativePtr = nativeLoadFdOffsets(format, fd, friendlyName, offset, length, flags, assets);
@@ -344,17 +331,16 @@ public final class ApkAssets {
    }

    private ApkAssets(@PropertyFlags int flags, @Nullable AssetsProvider assets) {
        this(FORMAT_APK, flags, assets, "empty");
        this(FORMAT_APK, flags, assets);
        mNativePtr = nativeLoadEmpty(flags, assets);
        mStringBlock = null;
    }

    private ApkAssets(@FormatType int format, @PropertyFlags int flags,
            @Nullable AssetsProvider assets, @NonNull String name) {
            @Nullable AssetsProvider assets) {
        mFlags = flags;
        mAssets = assets;
        mIsOverlay = format == FORMAT_IDMAP;
        if (DEBUG) mName = name;
    }

    @UnsupportedAppUsage
@@ -435,41 +421,13 @@ public final class ApkAssets {
        }
    }

    private static double intervalMs(long beginNs, long endNs) {
        return (endNs - beginNs) / 1000000.0;
    }

    /**
     * Returns false if the underlying APK was changed since this ApkAssets was loaded.
     */
    public boolean isUpToDate() {
        // This function is performance-critical - it's called multiple times on every Resources
        // object creation, and on few other cache accesses - so it's important to avoid the native
        // call when we know for sure what it will return (which is the case for both ALWAYS_TRUE
        // and FALSE).
        if (mPreviousUpToDateResult != UPTODATE_TRUE) {
            return mPreviousUpToDateResult == UPTODATE_ALWAYS_TRUE;
        }
        final long beforeTs, afterLockTs, afterNativeTs, afterUnlockTs;
        if (DEBUG) beforeTs = System.nanoTime();
        final int res;
        synchronized (this) {
            if (DEBUG) afterLockTs = System.nanoTime();
            res = nativeIsUpToDate(mNativePtr);
            if (DEBUG) afterNativeTs = System.nanoTime();
        }
        if (DEBUG) {
            afterUnlockTs = System.nanoTime();
            if (afterUnlockTs - beforeTs >= 10L * 1000000) {
                Log.d("ApkAssets", "isUpToDate(" + mName + ") took "
                        + intervalMs(beforeTs, afterUnlockTs)
                        + " ms: " + intervalMs(beforeTs, afterLockTs)
                        + " / " + intervalMs(afterLockTs, afterNativeTs)
                        + " / " + intervalMs(afterNativeTs, afterUnlockTs));
            }
            return nativeIsUpToDate(mNativePtr);
        }
        mPreviousUpToDateResult = res;
        return res != UPTODATE_FALSE;
    }

    public boolean isSystem() {
@@ -529,7 +487,7 @@ public final class ApkAssets {
    private static native @NonNull String nativeGetAssetPath(long ptr);
    private static native @NonNull String nativeGetDebugName(long ptr);
    private static native long nativeGetStringBlock(long ptr);
    @CriticalNative private static native int nativeIsUpToDate(long ptr);
    @CriticalNative private static native boolean nativeIsUpToDate(long ptr);
    private static native long nativeOpenXml(long ptr, @NonNull String fileName) throws IOException;
    private static native @Nullable OverlayableInfo nativeGetOverlayableInfo(long ptr,
            String overlayableName) throws IOException;
+5 −5
Original line number Diff line number Diff line
@@ -129,8 +129,8 @@ class LoaderAssetsProvider : public AssetsProvider {
    return debug_name_;
  }

  UpToDate IsUpToDate() const override {
      return UpToDate::Always;
  bool IsUpToDate() const override {
    return true;
  }

  ~LoaderAssetsProvider() override {
@@ -443,10 +443,10 @@ static jlong NativeGetStringBlock(JNIEnv* /*env*/, jclass /*clazz*/, jlong ptr)
    return reinterpret_cast<jlong>(apk_assets->GetLoadedArsc()->GetStringPool());
}

static jint NativeIsUpToDate(CRITICAL_JNI_PARAMS_COMMA jlong ptr) {
static jboolean NativeIsUpToDate(CRITICAL_JNI_PARAMS_COMMA jlong ptr) {
    auto scoped_apk_assets = ScopedLock(ApkAssetsFromLong(ptr));
    auto apk_assets = scoped_apk_assets->get();
    return (jint)apk_assets->IsUpToDate();
    return apk_assets->IsUpToDate() ? JNI_TRUE : JNI_FALSE;
}

static jlong NativeOpenXml(JNIEnv* env, jclass /*clazz*/, jlong ptr, jstring file_name) {
@@ -558,7 +558,7 @@ static const JNINativeMethod gApkAssetsMethods[] = {
        {"nativeGetDebugName", "(J)Ljava/lang/String;", (void*)NativeGetDebugName},
        {"nativeGetStringBlock", "(J)J", (void*)NativeGetStringBlock},
        // @CriticalNative
        {"nativeIsUpToDate", "(J)I", (void*)NativeIsUpToDate},
        {"nativeIsUpToDate", "(J)Z", (void*)NativeIsUpToDate},
        {"nativeOpenXml", "(JLjava/lang/String;)J", (void*)NativeOpenXml},
        {"nativeGetOverlayableInfo", "(JLjava/lang/String;)Landroid/content/om/OverlayableInfo;",
         (void*)NativeGetOverlayableInfo},
+3 −6
Original line number Diff line number Diff line
@@ -162,13 +162,10 @@ const std::string& ApkAssets::GetDebugName() const {
  return assets_provider_->GetDebugName();
}

UpToDate ApkAssets::IsUpToDate() const {
bool ApkAssets::IsUpToDate() const {
  // Loaders are invalidated by the app, not the system, so assume they are up to date.
  if (IsLoader()) {
    return UpToDate::Always;
  }
  const auto idmap_res = loaded_idmap_ ? loaded_idmap_->IsUpToDate() : UpToDate::Always;
  return combine(idmap_res, [this] { return assets_provider_->IsUpToDate(); });
  return IsLoader() || ((!loaded_idmap_ || loaded_idmap_->IsUpToDate())
                        && assets_provider_->IsUpToDate());
}

}  // namespace android
+39 −26
Original line number Diff line number Diff line
@@ -86,9 +86,11 @@ void ZipAssetsProvider::ZipCloser::operator()(ZipArchive* a) const {
}

ZipAssetsProvider::ZipAssetsProvider(ZipArchiveHandle handle, PathOrDebugName&& path,
                                     package_property_t flags, ModDate last_mod_time)
    : zip_handle_(handle), name_(std::move(path)), flags_(flags), last_mod_time_(last_mod_time) {
}
                                     package_property_t flags, time_t last_mod_time)
    : zip_handle_(handle),
      name_(std::move(path)),
      flags_(flags),
      last_mod_time_(last_mod_time) {}

std::unique_ptr<ZipAssetsProvider> ZipAssetsProvider::Create(std::string path,
                                                             package_property_t flags,
@@ -102,10 +104,10 @@ std::unique_ptr<ZipAssetsProvider> ZipAssetsProvider::Create(std::string path,
    return {};
  }

  ModDate mod_date = kInvalidModDate;
  struct stat sb{.st_mtime = -1};
  // Skip all up-to-date checks if the file won't ever change.
  if (isKnownWritablePath(path.c_str()) || !isReadonlyFilesystem(GetFileDescriptor(handle))) {
    if (mod_date = getFileModDate(GetFileDescriptor(handle)); mod_date == kInvalidModDate) {
  if (!isReadonlyFilesystem(path.c_str())) {
    if ((released_fd < 0 ? stat(path.c_str(), &sb) : fstat(released_fd, &sb)) < 0) {
      // Stat requires execute permissions on all directories path to the file. If the process does
      // not have execute permissions on this file, allow the zip to be opened but IsUpToDate() will
      // always have to return true.
@@ -114,7 +116,7 @@ std::unique_ptr<ZipAssetsProvider> ZipAssetsProvider::Create(std::string path,
  }

  return std::unique_ptr<ZipAssetsProvider>(
      new ZipAssetsProvider(handle, PathOrDebugName::Path(std::move(path)), flags, mod_date));
      new ZipAssetsProvider(handle, PathOrDebugName::Path(std::move(path)), flags, sb.st_mtime));
}

std::unique_ptr<ZipAssetsProvider> ZipAssetsProvider::Create(base::unique_fd fd,
@@ -135,10 +137,10 @@ std::unique_ptr<ZipAssetsProvider> ZipAssetsProvider::Create(base::unique_fd fd,
    return {};
  }

  ModDate mod_date = kInvalidModDate;
  struct stat sb{.st_mtime = -1};
  // Skip all up-to-date checks if the file won't ever change.
  if (!isReadonlyFilesystem(released_fd)) {
    if (mod_date = getFileModDate(released_fd); mod_date == kInvalidModDate) {
    if (fstat(released_fd, &sb) < 0) {
      // Stat requires execute permissions on all directories path to the file. If the process does
      // not have execute permissions on this file, allow the zip to be opened but IsUpToDate() will
      // always have to return true.
@@ -148,7 +150,7 @@ std::unique_ptr<ZipAssetsProvider> ZipAssetsProvider::Create(base::unique_fd fd,
  }

  return std::unique_ptr<ZipAssetsProvider>(new ZipAssetsProvider(
      handle, PathOrDebugName::DebugName(std::move(friendly_name)), flags, mod_date));
      handle, PathOrDebugName::DebugName(std::move(friendly_name)), flags, sb.st_mtime));
}

std::unique_ptr<Asset> ZipAssetsProvider::OpenInternal(const std::string& path,
@@ -280,16 +282,21 @@ const std::string& ZipAssetsProvider::GetDebugName() const {
  return name_.GetDebugName();
}

UpToDate ZipAssetsProvider::IsUpToDate() const {
  if (last_mod_time_ == kInvalidModDate) {
    return UpToDate::Always;
bool ZipAssetsProvider::IsUpToDate() const {
  if (last_mod_time_ == -1) {
    return true;
  }
  struct stat sb{};
  if (fstat(GetFileDescriptor(zip_handle_.get()), &sb) < 0) {
    // If fstat fails on the zip archive, return true so the zip archive the resource system does
    // attempt to refresh the ApkAsset.
    return true;
  }
  return fromBool(last_mod_time_ == getFileModDate(GetFileDescriptor(zip_handle_.get())));
  return last_mod_time_ == sb.st_mtime;
}

DirectoryAssetsProvider::DirectoryAssetsProvider(std::string&& path, ModDate last_mod_time)
    : dir_(std::move(path)), last_mod_time_(last_mod_time) {
}
DirectoryAssetsProvider::DirectoryAssetsProvider(std::string&& path, time_t last_mod_time)
    : dir_(std::move(path)), last_mod_time_(last_mod_time) {}

std::unique_ptr<DirectoryAssetsProvider> DirectoryAssetsProvider::Create(std::string path) {
  struct stat sb;
@@ -310,7 +317,7 @@ std::unique_ptr<DirectoryAssetsProvider> DirectoryAssetsProvider::Create(std::st

  const bool isReadonly = isReadonlyFilesystem(path.c_str());
  return std::unique_ptr<DirectoryAssetsProvider>(
      new DirectoryAssetsProvider(std::move(path), isReadonly ? kInvalidModDate : getModDate(sb)));
      new DirectoryAssetsProvider(std::move(path), isReadonly ? -1 : sb.st_mtime));
}

std::unique_ptr<Asset> DirectoryAssetsProvider::OpenInternal(const std::string& path,
@@ -339,11 +346,17 @@ const std::string& DirectoryAssetsProvider::GetDebugName() const {
  return dir_;
}

UpToDate DirectoryAssetsProvider::IsUpToDate() const {
  if (last_mod_time_ == kInvalidModDate) {
    return UpToDate::Always;
bool DirectoryAssetsProvider::IsUpToDate() const {
  if (last_mod_time_ == -1) {
    return true;
  }
  return fromBool(last_mod_time_ == getFileModDate(dir_.c_str()));
  struct stat sb;
  if (stat(dir_.c_str(), &sb) < 0) {
    // If stat fails on the zip archive, return true so the zip archive the resource system does
    // attempt to refresh the ApkAsset.
    return true;
  }
  return last_mod_time_ == sb.st_mtime;
}

MultiAssetsProvider::MultiAssetsProvider(std::unique_ptr<AssetsProvider>&& primary,
@@ -384,8 +397,8 @@ const std::string& MultiAssetsProvider::GetDebugName() const {
  return debug_name_;
}

UpToDate MultiAssetsProvider::IsUpToDate() const {
  return combine(primary_->IsUpToDate(), [this] { return secondary_->IsUpToDate(); });
bool MultiAssetsProvider::IsUpToDate() const {
  return primary_->IsUpToDate() && secondary_->IsUpToDate();
}

EmptyAssetsProvider::EmptyAssetsProvider(std::optional<std::string>&& path) :
@@ -429,8 +442,8 @@ const std::string& EmptyAssetsProvider::GetDebugName() const {
  return kEmpty;
}

UpToDate EmptyAssetsProvider::IsUpToDate() const {
  return UpToDate::Always;
bool EmptyAssetsProvider::IsUpToDate() const {
  return true;
}

}  // namespace android
+6 −15
Original line number Diff line number Diff line
@@ -22,10 +22,9 @@
#include "android-base/logging.h"
#include "android-base/stringprintf.h"
#include "android-base/utf8.h"
#include "androidfw/AssetManager.h"
#include "androidfw/misc.h"
#include "androidfw/ResourceTypes.h"
#include "androidfw/Util.h"
#include "androidfw/misc.h"
#include "utils/ByteOrder.h"
#include "utils/Trace.h"

@@ -269,16 +268,11 @@ LoadedIdmap::LoadedIdmap(const std::string& idmap_path, const Idmap_header* head
      configurations_(configs),
      overlay_entries_(overlay_entries),
      string_pool_(std::move(string_pool)),
      idmap_fd_(
          android::base::utf8::open(idmap_path.c_str(), O_RDONLY | O_CLOEXEC | O_BINARY | O_PATH)),
      overlay_apk_path_(overlay_apk_path),
      target_apk_path_(target_apk_path),
      idmap_last_mod_time_(kInvalidModDate) {
  if (!isReadonlyFilesystem(std::string(overlay_apk_path_).c_str()) ||
      !(target_apk_path_ == AssetManager::TARGET_APK_PATH ||
        isReadonlyFilesystem(std::string(target_apk_path_).c_str()))) {
    idmap_fd_.reset(
        android::base::utf8::open(idmap_path.c_str(), O_RDONLY | O_CLOEXEC | O_BINARY | O_PATH));
    idmap_last_mod_time_ = getFileModDate(idmap_fd_);
  }
      idmap_last_mod_time_(getFileModDate(idmap_fd_.get())) {
}

std::unique_ptr<LoadedIdmap> LoadedIdmap::Load(StringPiece idmap_path, StringPiece idmap_data) {
@@ -387,11 +381,8 @@ std::unique_ptr<LoadedIdmap> LoadedIdmap::Load(StringPiece idmap_path, StringPie
                      overlay_entries, std::move(idmap_string_pool), *overlay_path, *target_path));
}

UpToDate LoadedIdmap::IsUpToDate() const {
  if (idmap_last_mod_time_ == kInvalidModDate) {
    return UpToDate::Always;
  }
  return fromBool(idmap_last_mod_time_ == getFileModDate(idmap_fd_.get()));
bool LoadedIdmap::IsUpToDate() const {
  return idmap_last_mod_time_ == getFileModDate(idmap_fd_.get());
}

}  // namespace android
Loading