Take a StringPiece when looking up CommandLine switches.

base/command_line.h

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 // This class works with command lines: building and parsing.
 // Arguments with prefixes ('--', '-', and on Windows, '/') are switches.
 // Switches will precede all other arguments without switch prefixes.
 // Switches can optionally have values, delimited by '=', e.g., "-switch=value".
 // An argument of "--" will terminate switch parsing during initialization,
 // interpreting subsequent tokens as non-switch arguments, regardless of prefix.
 
 // There is a singleton read-only CommandLine that represents the command line
 // that the current process was started with.  It must be initialized in main().
 
 #ifndef BASE_COMMAND_LINE_H_
 #define BASE_COMMAND_LINE_H_
 
 #include <stddef.h>
 #include <map>
 #include <string>
 #include <vector>
 
 #include "base/base_export.h"
 #include "base/strings/string16.h"
+#include "base/strings/string_piece.h"
 #include "build/build_config.h"
 
 namespace base {
 
 class FilePath;
 
 class BASE_EXPORT CommandLine {
  public:
 #if defined(OS_WIN)
   // The native command line string type.
@@ skipping 10 matching lines @@
   enum NoProgram { NO_PROGRAM };
   explicit CommandLine(NoProgram no_program);
 
   // Construct a new command line with |program| as argv[0].
   explicit CommandLine(const FilePath& program);
 
   // Construct a new command line from an argument list.
   CommandLine(int argc, const CharType* const* argv);
   explicit CommandLine(const StringVector& argv);
 
+  // Override copy and assign to ensure |switches_by_stringpiece_| is valid.
+  CommandLine(const CommandLine& other);

[tapted, 2015/04/15 00:07:53]

it's tempting to make this explicit... I bet a lot of calls are unintentionally
copying this by value when they can just take a reference instead. That's for a
follow-up in any case.

+  CommandLine& operator=(const CommandLine& other);
+
   ~CommandLine();
 
 #if defined(OS_WIN)
   // By default this class will treat command-line arguments beginning with
   // slashes as switches on Windows, but not other platforms.
   //
   // If this behavior is inappropriate for your application, you can call this
   // function BEFORE initializing the current process' global command line
   // object and the behavior will be the same as Posix systems (only hyphens
   // begin switches, everything else will be an arg).
@@ skipping 70 matching lines @@
 #endif
 
   // Returns the original command line string as a vector of strings.
   const StringVector& argv() const { return argv_; }
 
   // Get and Set the program part of the command line string (the first item).
   FilePath GetProgram() const;
   void SetProgram(const FilePath& program);
 
   // Returns true if this command line contains the given switch.
-  // (Switch names are case-insensitive).
+  // Switch names should only be lowercase.

[tapted, 2015/04/15 00:07:53]

nit: should only -> must

[jackhou1, 2015/04/17 00:56:10]

Done.

   // The second override provides an optimized version to avoid inlining the
   // codegen for the string allocation.

[tapted, 2015/04/15 00:07:53]

This last sentence in the comment is no longer valid. For a StringPiece, there
is still codegen for the strlen and the stack allocation of the StringPiece,
versus just passing a pointer to the char* in a register. I'd expect a ~24kB hit
to sizes by removing the overload.

I'd suggest keeping the overload, but updating this comment to say something
like "... to avoid inlining the codegen to find the length of the constant and
construct a StringPiece."

[jackhou1, 2015/04/17 00:56:10]

Done.

-  bool HasSwitch(const std::string& switch_string) const;
-  bool HasSwitch(const char switch_constant[]) const;
+  bool HasSwitch(const base::StringPiece& switch_string) const;
 
   // Returns the value associated with the given switch. If the switch has no
   // value or isn't present, this method returns the empty string.
-  std::string GetSwitchValueASCII(const std::string& switch_string) const;
-  FilePath GetSwitchValuePath(const std::string& switch_string) const;
-  StringType GetSwitchValueNative(const std::string& switch_string) const;
+  // Switch names should only be lowercase.
+  std::string GetSwitchValueASCII(const base::StringPiece& switch_string) const;
+  FilePath GetSwitchValuePath(const base::StringPiece& switch_string) const;
+  StringType GetSwitchValueNative(const base::StringPiece& switch_string) const;
 
-  // Get a copy of all switches, along with their values.
+  // Get a reference to the internal switch map.
   const SwitchMap& GetSwitches() const { return switches_; }
 
   // Append a switch [with optional value] to the command line.
   // Note: Switches will precede arguments regardless of appending order.
   void AppendSwitch(const std::string& switch_string);
   void AppendSwitchPath(const std::string& switch_string,
                         const FilePath& path);
   void AppendSwitchNative(const std::string& switch_string,
                           const StringType& value);
   void AppendSwitchASCII(const std::string& switch_string,
@@ skipping 48 matching lines @@
 
   // The singleton CommandLine representing the current process's command line.
   static CommandLine* current_process_commandline_;
 
   // The argv array: { program, [(--|-|/)switch[=value]]*, [--], [argument]* }
   StringVector argv_;
 
   // Parsed-out switch keys and values.
   SwitchMap switches_;
 
+  // A mirror of |switches_| with only references to the actual strings.
+  // Used for allocation-free lookups.

[tapted, 2015/04/15 00:07:53]

nit: mention mapped_type points to an object in |switches_|

[jackhou1, 2015/04/17 00:56:10]

Done.

+  std::map<base::StringPiece, const StringType*> switches_by_stringpiece_;

[tapted, 2015/04/15 00:07:53]

It might be less confusing to just store a value for the mapped type - it would
be good to have numbers on the benefit of avoiding the extra copies

[jackhou1, 2015/04/17 00:56:10]

I'm think the memory/performance impact would be negligible since CommandLines
typically have very few switches. But I also think the impact on code clarity of
storing a pointer is not that big. Since the key is basically a pointer into
|switches| it seems natural that the mapped_type is as well.

But I don't mind either way if you feel strongly about it.

+
   // The index after the program and switches, any arguments start here.
   size_t begin_args_;
 };
 
 }  // namespace base
 
 #endif  // BASE_COMMAND_LINE_H_