Enabled AtomMatcherOptimization by default

Bug: 233279770
Test: build & statsd_test

Change-Id: I0a9bff00764a4ccce1d37c35c6681f24611ab489
Merged-In: I0a9bff00764a4ccce1d37c35c6681f24611ab489

6 files changed, 20 insertions, 118 deletions

diff --git a/statsd/src/flags/FlagProvider.h b/statsd/src/flags/FlagProvider.h
index e948911f..7e11febe 100644
--- a/statsd/src/flags/FlagProvider.h
+++ b/statsd/src/flags/FlagProvider.h
@@ -37,8 +37,6 @@ using IsAtLeastSFunc = std::function<bool()>;
 const std::string STATSD_NATIVE_NAMESPACE = "statsd_native";
 const std::string STATSD_NATIVE_BOOT_NAMESPACE = "statsd_native_boot";
 
-const std::string OPTIMIZATION_ATOM_MATCHER_MAP_FLAG = "optimization_atom_matcher_map";
-
 const std::string LIMIT_PULL_FLAG = "limit_pull";
 
 const std::string FLAG_TRUE = "true";
@@ -129,7 +127,6 @@ private:
     FRIEND_TEST(FlagProviderTest_SPlus, TestGetFlagBoolServerFlagEmptyDefaultTrue);
     FRIEND_TEST(FlagProviderTest_SPlus_RealValues, TestGetBootFlagBoolServerFlagTrue);
     FRIEND_TEST(FlagProviderTest_SPlus_RealValues, TestGetBootFlagBoolServerFlagFalse);
-    FRIEND_TEST(MetricsManagerTest_SPlus, TestAtomMatcherOptimizationEnabledFlag);
 };
 
 }  // namespace statsd
diff --git a/statsd/src/main.cpp b/statsd/src/main.cpp
index 02dbbe84..367ab733 100644
--- a/statsd/src/main.cpp
+++ b/statsd/src/main.cpp
@@ -76,8 +76,7 @@ int main(int /*argc*/, char** /*argv*/) {
             std::make_shared<LogEventQueue>(4000 /*buffer limit. Buffer is NOT pre-allocated*/);
 
     // Initialize boot flags
-    FlagProvider::getInstance().initBootFlags(
-            {OPTIMIZATION_ATOM_MATCHER_MAP_FLAG, LIMIT_PULL_FLAG});
+    FlagProvider::getInstance().initBootFlags({LIMIT_PULL_FLAG});
 
     sp<UidMap> uidMap = UidMap::getInstance();
 
diff --git a/statsd/src/metrics/MetricsManager.cpp b/statsd/src/metrics/MetricsManager.cpp
index 156d7c21..361cb1a8 100644
--- a/statsd/src/metrics/MetricsManager.cpp
+++ b/statsd/src/metrics/MetricsManager.cpp
@@ -76,9 +76,7 @@ MetricsManager::MetricsManager(const ConfigKey& key, const StatsdConfig& config,
       mPullerManager(pullerManager),
       mWhitelistedAtomIds(config.whitelisted_atom_ids().begin(),
                           config.whitelisted_atom_ids().end()),
-      mShouldPersistHistory(config.persist_locally()),
-      mAtomMatcherOptimizationEnabled(FlagProvider::getInstance().getBootFlagBool(
-              OPTIMIZATION_ATOM_MATCHER_MAP_FLAG, FLAG_FALSE)) {
+      mShouldPersistHistory(config.persist_locally()) {
     // Init the ttl end timestamp.
     refreshTtl(timeBaseNs);
 
@@ -578,16 +576,9 @@ void MetricsManager::onLogEvent(const LogEvent& event) {
     vector<MatchingState> matcherCache(mAllAtomMatchingTrackers.size(),
                                        MatchingState::kNotComputed);
 
-    if (mAtomMatcherOptimizationEnabled) {
-        for (const auto& matcherIndex : matchersIt->second) {
-            mAllAtomMatchingTrackers[matcherIndex]->onLogEvent(event, mAllAtomMatchingTrackers,
-                                                               matcherCache);
-        }
-    } else {
-        //  Evaluate all atom matchers.
-        for (auto& matcher : mAllAtomMatchingTrackers) {
-            matcher->onLogEvent(event, mAllAtomMatchingTrackers, matcherCache);
-        }
+    for (const auto& matcherIndex : matchersIt->second) {
+        mAllAtomMatchingTrackers[matcherIndex]->onLogEvent(event, mAllAtomMatchingTrackers,
+                                                           matcherCache);
     }
 
     // Set of metrics that received an activation cancellation.
diff --git a/statsd/src/metrics/MetricsManager.h b/statsd/src/metrics/MetricsManager.h
index 527efd9b..5b9731ed 100644
--- a/statsd/src/metrics/MetricsManager.h
+++ b/statsd/src/metrics/MetricsManager.h
@@ -217,8 +217,6 @@ private:
 
     bool mShouldPersistHistory;
 
-    const bool mAtomMatcherOptimizationEnabled;
-
     // All event tags that are interesting to config metrics matchers.
     std::unordered_map<int, std::vector<int>> mTagIdsToMatchersMap;
 
@@ -354,7 +352,6 @@ private:
 
     FRIEND_TEST(MetricsManagerTest, TestLogSources);
     FRIEND_TEST(MetricsManagerTest, TestLogSourcesOnConfigUpdate);
-    FRIEND_TEST(MetricsManagerTest_SPlus, TestAtomMatcherOptimizationEnabledFlag);
     FRIEND_TEST(MetricsManagerUtilTest, TestSampledMetrics);
 
     FRIEND_TEST(StatsLogProcessorTest, TestActiveConfigMetricDiskWriteRead);
diff --git a/statsd/tests/MetricsManager_test.cpp b/statsd/tests/MetricsManager_test.cpp
index 9650f2d6..73e3d3ce 100644
--- a/statsd/tests/MetricsManager_test.cpp
+++ b/statsd/tests/MetricsManager_test.cpp
@@ -265,73 +265,6 @@ TEST(MetricsManagerTest, TestLogSourcesOnConfigUpdate) {
                 UnorderedElementsAreArray(unionSet({defaultPullUids, app2Uids, {AID_ADB}})));
 }
 
-struct MetricsManagerServerFlagParam {
-    string flagValue;
-    string label;
-};
-
-class MetricsManagerTest_SPlus
-    : public ::testing::Test,
-      public ::testing::WithParamInterface<MetricsManagerServerFlagParam> {
-protected:
-    void SetUp() override {
-        if (shouldSkipTest()) {
-            GTEST_SKIP() << skipReason();
-        }
-
-        originalFlagValue = FlagProvider::getInstance().getFlagString(
-                OPTIMIZATION_ATOM_MATCHER_MAP_FLAG, FLAG_EMPTY);
-    }
-
-    bool shouldSkipTest() const {
-        return !IsAtLeastS();
-    }
-
-    string skipReason() const {
-        return "Skipping MetricsManagerTest_SPlus because device is not S+";
-    }
-
-    void TearDown() override {
-        if (originalFlagValue) {
-            writeFlag(OPTIMIZATION_ATOM_MATCHER_MAP_FLAG, originalFlagValue.value());
-        }
-    }
-
-    std::optional<string> originalFlagValue;
-};
-
-INSTANTIATE_TEST_SUITE_P(
-        MetricsManagerTest_SPlus, MetricsManagerTest_SPlus,
-        testing::ValuesIn<MetricsManagerServerFlagParam>({
-                // Server flag values
-                {FLAG_TRUE, "ServerFlagTrue"},
-                {FLAG_FALSE, "ServerFlagFalse"},
-        }),
-        [](const testing::TestParamInfo<MetricsManagerTest_SPlus::ParamType>& info) {
-            return info.param.label;
-        });
-
-TEST_P(MetricsManagerTest_SPlus, TestAtomMatcherOptimizationEnabledFlag) {
-    FlagProvider::getInstance().overrideFlag(OPTIMIZATION_ATOM_MATCHER_MAP_FLAG,
-                                             GetParam().flagValue,
-                                             /*isBootFlag=*/true);
-
-    sp<UidMap> uidMap;
-    sp<StatsPullerManager> pullerManager = new StatsPullerManager();
-    sp<AlarmMonitor> anomalyAlarmMonitor;
-    sp<AlarmMonitor> periodicAlarmMonitor;
-
-    StatsdConfig config = buildGoodConfig();
-    MetricsManager metricsManager(kConfigKey, config, timeBaseSec, timeBaseSec, uidMap,
-                                  pullerManager, anomalyAlarmMonitor, periodicAlarmMonitor);
-
-    if (GetParam().flagValue == FLAG_TRUE) {
-        EXPECT_TRUE(metricsManager.mAtomMatcherOptimizationEnabled);
-    } else {
-        EXPECT_FALSE(metricsManager.mAtomMatcherOptimizationEnabled);
-    }
-}
-
 TEST(MetricsManagerTest, TestCheckLogCredentialsWhitelistedAtom) {
     sp<UidMap> uidMap;
     sp<StatsPullerManager> pullerManager = new StatsPullerManager();
diff --git a/statsd/tests/StatsLogProcessor_test.cpp b/statsd/tests/StatsLogProcessor_test.cpp
index 32f091f7..019584e3 100644
--- a/statsd/tests/StatsLogProcessor_test.cpp
+++ b/statsd/tests/StatsLogProcessor_test.cpp
@@ -66,22 +66,7 @@ public:
     MOCK_METHOD1(dropData, void(const int64_t dropTimeNs));
 };
 
-// Setup for test fixture.
-class StatsLogProcessorTest : public testing::TestWithParam<string> {
-    void SetUp() override {
-        FlagProvider::getInstance().overrideFlag(OPTIMIZATION_ATOM_MATCHER_MAP_FLAG, GetParam(),
-                                                 /*isBootFlag=*/true);
-    }
-
-    void TearDown() override {
-        FlagProvider::getInstance().resetOverrides();
-    }
-};
-
-INSTANTIATE_TEST_SUITE_P(OptimizationAtomMatcher, StatsLogProcessorTest,
-                         testing::Values(FLAG_FALSE, FLAG_TRUE));
-
-TEST_P(StatsLogProcessorTest, TestRateLimitByteSize) {
+TEST(StatsLogProcessorTest, TestRateLimitByteSize) {
     sp<UidMap> m = new UidMap();
     sp<StatsPullerManager> pullerManager = new StatsPullerManager();
     sp<AlarmMonitor> anomalyAlarmMonitor;
@@ -102,7 +87,7 @@ TEST_P(StatsLogProcessorTest, TestRateLimitByteSize) {
     p.flushIfNecessaryLocked(key, mockMetricsManager);
 }
 
-TEST_P(StatsLogProcessorTest, TestRateLimitBroadcast) {
+TEST(StatsLogProcessorTest, TestRateLimitBroadcast) {
     sp<UidMap> m = new UidMap();
     sp<StatsPullerManager> pullerManager = new StatsPullerManager();
     sp<AlarmMonitor> anomalyAlarmMonitor;
@@ -134,7 +119,7 @@ TEST_P(StatsLogProcessorTest, TestRateLimitBroadcast) {
     // EXPECT_EQ(1, broadcastCount);
 }
 
-TEST_P(StatsLogProcessorTest, TestDropWhenByteSizeTooLarge) {
+TEST(StatsLogProcessorTest, TestDropWhenByteSizeTooLarge) {
     sp<UidMap> m = new UidMap();
     sp<StatsPullerManager> pullerManager = new StatsPullerManager();
     sp<AlarmMonitor> anomalyAlarmMonitor;
@@ -176,7 +161,7 @@ StatsdConfig MakeConfig(bool includeMetric) {
     return config;
 }
 
-TEST_P(StatsLogProcessorTest, TestUidMapHasSnapshot) {
+TEST(StatsLogProcessorTest, TestUidMapHasSnapshot) {
     // Setup simple config key corresponding to empty config.
     sp<UidMap> m = new UidMap();
     sp<StatsPullerManager> pullerManager = new StatsPullerManager();
@@ -208,7 +193,7 @@ TEST_P(StatsLogProcessorTest, TestUidMapHasSnapshot) {
     ASSERT_EQ(2, uidmap.snapshots(0).package_info_size());
 }
 
-TEST_P(StatsLogProcessorTest, TestEmptyConfigHasNoUidMap) {
+TEST(StatsLogProcessorTest, TestEmptyConfigHasNoUidMap) {
     // Setup simple config key corresponding to empty config.
     sp<UidMap> m = new UidMap();
     sp<StatsPullerManager> pullerManager = new StatsPullerManager();
@@ -238,7 +223,7 @@ TEST_P(StatsLogProcessorTest, TestEmptyConfigHasNoUidMap) {
     EXPECT_FALSE(output.reports(0).has_uid_map());
 }
 
-TEST_P(StatsLogProcessorTest, TestReportIncludesSubConfig) {
+TEST(StatsLogProcessorTest, TestReportIncludesSubConfig) {
     // Setup simple config key corresponding to empty config.
     sp<UidMap> m = new UidMap();
     sp<StatsPullerManager> pullerManager = new StatsPullerManager();
@@ -272,7 +257,7 @@ TEST_P(StatsLogProcessorTest, TestReportIncludesSubConfig) {
     EXPECT_EQ(2, report.annotation(0).field_int32());
 }
 
-TEST_P(StatsLogProcessorTest, TestOnDumpReportEraseData) {
+TEST(StatsLogProcessorTest, TestOnDumpReportEraseData) {
     // Setup a simple config.
     StatsdConfig config;
     config.add_allowed_log_source("AID_ROOT");  // LogEvent defaults to UID of root.
@@ -320,7 +305,7 @@ TEST_P(StatsLogProcessorTest, TestOnDumpReportEraseData) {
     EXPECT_TRUE(noData);
 }
 
-TEST_P(StatsLogProcessorTest, TestPullUidProviderSetOnConfigUpdate) {
+TEST(StatsLogProcessorTest, TestPullUidProviderSetOnConfigUpdate) {
     // Setup simple config key corresponding to empty config.
     sp<UidMap> m = new UidMap();
     sp<StatsPullerManager> pullerManager = new StatsPullerManager();
@@ -343,7 +328,7 @@ TEST_P(StatsLogProcessorTest, TestPullUidProviderSetOnConfigUpdate) {
     EXPECT_EQ(pullerManager->mPullUidProviders.find(key), pullerManager->mPullUidProviders.end());
 }
 
-TEST_P(StatsLogProcessorTest, InvalidConfigRemoved) {
+TEST(StatsLogProcessorTest, InvalidConfigRemoved) {
     // Setup simple config key corresponding to empty config.
     sp<UidMap> m = new UidMap();
     sp<StatsPullerManager> pullerManager = new StatsPullerManager();
@@ -381,7 +366,7 @@ TEST_P(StatsLogProcessorTest, InvalidConfigRemoved) {
     StorageManager::deleteSuffixedFiles(STATS_DATA_DIR, suffix.c_str());
 }
 
-TEST_P(StatsLogProcessorTest, TestActiveConfigMetricDiskWriteRead) {
+TEST(StatsLogProcessorTest, TestActiveConfigMetricDiskWriteRead) {
     int uid = 1111;
 
     // Setup a simple config, no activation
@@ -726,7 +711,7 @@ TEST_P(StatsLogProcessorTest, TestActiveConfigMetricDiskWriteRead) {
     EXPECT_EQ(broadcastCount, 1);
 }
 
-TEST_P(StatsLogProcessorTest, TestActivationOnBoot) {
+TEST(StatsLogProcessorTest, TestActivationOnBoot) {
     int uid = 1111;
 
     StatsdConfig config1;
@@ -847,7 +832,7 @@ TEST_P(StatsLogProcessorTest, TestActivationOnBoot) {
     EXPECT_EQ(kActive, activation1001->state);
 }
 
-TEST_P(StatsLogProcessorTest, TestActivationOnBootMultipleActivations) {
+TEST(StatsLogProcessorTest, TestActivationOnBootMultipleActivations) {
     int uid = 1111;
 
     // Create config with 2 metrics:
@@ -1248,7 +1233,7 @@ TEST_P(StatsLogProcessorTest, TestActivationOnBootMultipleActivations) {
     // }}}-------------------------------------------------------------------------------
 }
 
-TEST_P(StatsLogProcessorTest, TestActivationOnBootMultipleActivationsDifferentActivationTypes) {
+TEST(StatsLogProcessorTest, TestActivationOnBootMultipleActivationsDifferentActivationTypes) {
     int uid = 1111;
 
     // Create config with 2 metrics:
@@ -1510,7 +1495,7 @@ TEST_P(StatsLogProcessorTest, TestActivationOnBootMultipleActivationsDifferentAc
     // }}}---------------------------------------------------------------------------
 }
 
-TEST_P(StatsLogProcessorTest, TestActivationsPersistAcrossSystemServerRestart) {
+TEST(StatsLogProcessorTest, TestActivationsPersistAcrossSystemServerRestart) {
     int uid = 9876;
     long configId = 12341;
 
@@ -1975,7 +1960,7 @@ TEST(StatsLogProcessorTest_mapIsolatedUidToHostUid, LogRepeatedUidField) {
     EXPECT_EQ(hostUid1, actualFieldValues->at(3).mValue.int_value);
 }
 
-TEST_P(StatsLogProcessorTest, TestDumpReportWithoutErasingDataDoesNotUpdateTimestamp) {
+TEST(StatsLogProcessorTest, TestDumpReportWithoutErasingDataDoesNotUpdateTimestamp) {
     int hostUid = 20;
     int isolatedUid = 30;
     sp<MockUidMap> mockUidMap = makeMockUidMapForHosts({{hostUid, {isolatedUid}}});