Fix issue with default GPU activity set

Summary:
D24157062 (b744449) uses Config::validate() to set the default set of GPU activity types if none has been specified. Problem is that validate is not being called for the activity profiler.

This fix makes validate() a virtual function in AbstractConfig and calls it from parse() before returning.

Differential Revision: D24225040

fbshipit-source-id: 616ba3c457970d15363591a12c7be746ffdfec01

Author: gdankel

libkineto/src/AbstractConfig.cpp

@@ -168,6 +168,8 @@ bool AbstractConfig::parse(const string& conf) {
     }
   }
 
+  validate();
+
   // Store original text, used to detect updates
   source_ = conf;
   timestamp_ = system_clock::now();

libkineto/src/AbstractConfig.h

@@ -58,6 +58,11 @@ class AbstractConfig {
   // Throw std::invalid_argument if val is invalid.
   virtual bool handleOption(const std::string& name, std::string& val);
 
+  // Perform post-validation checks, typically conditons involving
+  // multiple options.
+  // Throw std::invalid_argument if automatic correction can not be made.
+  virtual void validate() = 0;
+
   // TODO: Separate out each profiler type into features?
   virtual void printActivityProfilerConfig(std::ostream& s) const;
 

libkineto/src/Config.h

@@ -244,7 +244,7 @@ class Config : public AbstractConfig {
 
   void printActivityProfilerConfig(std::ostream& s) const override;
 
-  void validate();
+  void validate() override;
 
   static void addConfigFactory(
       std::string name,

libkineto/test/ConfigTest.cpp

@@ -86,24 +86,32 @@ TEST(ParseTest, ActivityTypes) {
   EXPECT_TRUE(cfg.parse("ACTIVITY_TYPES="));
   EXPECT_FALSE(cfg.parse("=ACTIVITY_TYPES="));
 
-  EXPECT_TRUE(cfg.parse("ACTIVITY_TYPES=gpu_memcpy,gpu_MeMsEt,concurrent_kernel"));
   EXPECT_EQ(cfg.selectedActivityTypes(),
+    std::set<ActivityType>({ActivityType::MEMCPY,
+                            ActivityType::MEMSET,
+                            ActivityType::CONCURRENT_KERNEL,
+                            ActivityType::EXTERNAL_CORRELATION,
+                            ActivityType::RUNTIME}));
+
+  Config cfg2;
+  EXPECT_TRUE(cfg2.parse("ACTIVITY_TYPES=gpu_memcpy,gpu_MeMsEt,concurrent_kernel"));
+  EXPECT_EQ(cfg2.selectedActivityTypes(),
     std::set<ActivityType>({ActivityType::MEMCPY,
                             ActivityType::MEMSET,
                             ActivityType::CONCURRENT_KERNEL}));
 
-  EXPECT_TRUE(cfg.parse("ACTIVITY_TYPES = cuda_Runtime,"));
-  EXPECT_EQ(cfg.selectedActivityTypes(),
+  EXPECT_TRUE(cfg2.parse("ACTIVITY_TYPES = cuda_Runtime,"));
+  EXPECT_EQ(cfg2.selectedActivityTypes(),
     std::set<ActivityType>({ActivityType::MEMCPY,
                             ActivityType::MEMSET,
                             ActivityType::RUNTIME,
                             ActivityType::CONCURRENT_KERNEL}));
 
   // Should throw an exception because incorrect activity name
-  EXPECT_FALSE(cfg.parse("ACTIVITY_TYPES = memcopy,cuda_runtime"));
+  EXPECT_FALSE(cfg2.parse("ACTIVITY_TYPES = memcopy,cuda_runtime"));
 
-  EXPECT_TRUE(cfg.parse("ACTIVITY_TYPES = external_correlation"));
-  EXPECT_EQ(cfg.selectedActivityTypes(),
+  EXPECT_TRUE(cfg2.parse("ACTIVITY_TYPES = external_correlation"));
+  EXPECT_EQ(cfg2.selectedActivityTypes(),
     std::set<ActivityType>({ActivityType::MEMCPY,
                             ActivityType::MEMSET,
                             ActivityType::CONCURRENT_KERNEL,