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

Commit b2bac12c authored by Mohamed Heikal's avatar Mohamed Heikal
Browse files

ResourcePathShortener deterministically handles hash collisions 

ResourcePathShortener appends one digit at the end of colliding short
file names to disambiguate them. This cl sorts the list of file paths so
that colliding files always get the same disambiguating digit from one
run to the next rather than possibly alternating.

Test: make aapt2_tests

Change-Id: I983a1448c21f2c79d7cdb1de232e7758c04fc256
parent c7487391

tools/aapt2/optimize/ResourcePathShortener.cpp

+11 −1
Original line number Diff line number Diff line
@@ -16,6 +16,7 @@ 


#include "optimize/ResourcePathShortener.h"



#include <set>

#include <unordered_set>



#include "androidfw/StringPiece.h"
@@ -71,10 +72,19 @@ std::string GetShortenedPath(const android::StringPiece& shortened_filename, 
  return shortened_path;

}



// implement custom comparator of FileReference pointers so as to use the

// underlying filepath as key rather than the integer address. This is to ensure

// determinism of output for colliding files.

struct PathComparator {

    bool operator() (const FileReference* lhs, const FileReference* rhs) const {

        return lhs->path->compare(*rhs->path);

    }

};



bool ResourcePathShortener::Consume(IAaptContext* context, ResourceTable* table) {

  // used to detect collisions

  std::unordered_set<std::string> shortened_paths;

  std::unordered_set<FileReference*> file_refs;

  std::set<FileReference*, PathComparator> file_refs;

  for (auto& package : table->packages) {

    for (auto& type : package->types) {

      for (auto& entry : type->entries) {

tools/aapt2/optimize/ResourcePathShortener_test.cpp

+49 −0
Original line number Diff line number Diff line
@@ -29,6 +29,14 @@ android::StringPiece GetExtension(android::StringPiece path) { 
  return android::StringPiece(iter, path.end() - iter);

}



void FillTable(aapt::test::ResourceTableBuilder& builder, int start, int end) {

  for (int i=start; i<end; i++) {

    builder.AddFileReference(

        "android:drawable/xmlfile" + std::to_string(i),

        "res/drawable/xmlfile" + std::to_string(i) + ".xml");

  }

}



namespace aapt {



TEST(ResourcePathShortenerTest, FileRefPathsChangedInResourceTable) {
@@ -114,4 +122,45 @@ TEST(ResourcePathShortenerTest, KeepExtensions) { 
  EXPECT_THAT(GetExtension(path_map[original_png_path]), Eq(android::StringPiece(".png")));

}



TEST(ResourcePathShortenerTest, DeterministicallyHandleCollisions) {

  std::unique_ptr<IAaptContext> context = test::ContextBuilder().Build();



  // 4000 resources is the limit at which the hash space is expanded to 3

  // letters to reduce collisions, we want as many collisions as possible thus

  // N-1.

  const auto kNumResources = 3999;

  const auto kNumTries = 5;



  test::ResourceTableBuilder builder1;

  FillTable(builder1, 0, kNumResources);

  std::unique_ptr<ResourceTable> table1 = builder1.Build();

  std::map<std::string, std::string> expected_mapping;

  ASSERT_TRUE(ResourcePathShortener(expected_mapping).Consume(context.get(), table1.get()));



  // We are trying to ensure lack of non-determinism, it is not simple to prove

  // a negative, thus we must try the test a few times so that the test itself

  // is non-flaky. Basically create the pathmap 5 times from the same set of

  // resources but a different order of addition and then ensure they are always

  // mapped to the same short path.

  for (int i=0; i<kNumTries; i++) {

    test::ResourceTableBuilder builder2;

    // This loop adds resources to the resource table in the range of

    // [0:kNumResources).  Adding the file references in different order makes

    // non-determinism more likely to surface. Thus we add resources

    // [start_index:kNumResources) first then [0:start_index). We also use a

    // different start_index each run.

    int start_index = (kNumResources/kNumTries)*i;

    FillTable(builder2, start_index, kNumResources);

    FillTable(builder2, 0, start_index);

    std::unique_ptr<ResourceTable> table2 = builder2.Build();



    std::map<std::string, std::string> actual_mapping;

    ASSERT_TRUE(ResourcePathShortener(actual_mapping).Consume(context.get(), table2.get()));



    for (auto& item : actual_mapping) {

      ASSERT_THAT(expected_mapping[item.first], Eq(item.second));

    }

  }

}



}   // namespace aapt