Merge remote-tracking branch 'origin/master' into wip/havocp-resolve-fixes

README.md

@@ -1,5 +1,8 @@
 Configuration library for JVM languages.
 
+[![Maven Central](https://maven-badges.herokuapp.com/maven-central/com.typesafe/config/badge.svg)](https://maven-badges.herokuapp.com/maven-central/com.typesafe/config)
+
+
 ## Overview
 
  - implemented in plain Java with no dependencies
@@ -89,9 +92,9 @@ Maven Central.
         <version>1.2.1</version>
     </dependency>
 
-Obsolete releases are here, but you probably don't want these:
+Link for direct download if you don't use a dependency manager:
 
- - http://repo.typesafe.com/typesafe/releases/com/typesafe/config/config/
+ - http://central.maven.org/maven2/com/typesafe/config/
 
 ### Release Notes
 
@@ -303,10 +306,13 @@ options:
 
  1. Set it in a `reference.conf` included in your library or
  application jar, so there's a default value.
- 2. Catch and handle `ConfigException.Missing`.
- 3. Use the `Config.hasPath()` method to check in advance whether
+ 2. Use the `Config.hasPath()` method to check in advance whether
  the path exists (rather than checking for `null`/`None` after as
  you might in other APIs).
+ 3. Catch and handle `ConfigException.Missing`. NOTE: using an
+ exception for control flow like this is much slower than using
+ `Config.hasPath()`; the JVM has to do a lot of work to throw
+ an exception.
  4. In your initialization code, generate a `Config` with your
  defaults in it (using something like `ConfigFactory.parseMap()`)
  then fold that default config into your loaded config using
@@ -318,7 +324,7 @@ options:
  `Config`; `ConfigObject` implements `java.util.Map<String,?>` and
  the `get()` method on `Map` returns null for missing keys. See
  the API docs for more detail on `Config` vs. `ConfigObject`.
- 
+
 The *recommended* path (for most cases, in most apps) is that you
 require all settings to be present in either `reference.conf` or
 `application.conf` and allow `ConfigException.Missing` to be
@@ -338,11 +344,10 @@ like this to use the idiomatic `Option` syntax:
 
 ```scala
 implicit class RichConfig(val underlying: Config) extends AnyVal {
-  def getOptionalBoolean(path: String): Option[Boolean] = try {
+  def getOptionalBoolean(path: String): Option[Boolean] = if (underlying.hasPath(path)) {
      Some(underlying.getBoolean(path))
-  } catch {
-     case e: ConfigException.Missing =>
-         None
+  } else {
+     None
   }
 }
 ```
@@ -564,6 +569,14 @@ You can take advantage of this for "inheritance":
 Using `include` statements you could split this across multiple
 files, too.
 
+If you put two objects next to each other (close brace of the first
+on the same line with open brace of the second), they are merged, so
+a shorter way to write the above "inheritance" example would be:
+
+    data-center-generic = { cluster-size = 6 }
+    data-center-east = ${data-center-generic} { name = "east" }
+    data-center-west = ${data-center-generic} { name = "west", cluster-size = 8 }
+
 #### Optional system or env variable overrides
 
 In default uses of the library, exact-match system properties

config/src/main/java/com/typesafe/config/ConfigRenderOptions.java

@@ -115,7 +115,7 @@ public final class ConfigRenderOptions {
      * whitespace, enabling formatting makes things prettier but larger.
      *
      * @param value
-     *            true to include comments in the render
+     *            true to enable formatting
      * @return options with requested setting for formatting
      */
     public ConfigRenderOptions setFormatted(boolean value) {
@@ -129,7 +129,7 @@ public final class ConfigRenderOptions {
      * Returns whether the options enable formatting. This method is mostly used
      * by the config lib internally, not by applications.
      *
-     * @return true if comments should be rendered
+     * @return true if the options enable formatting
      */
     public boolean getFormatted() {
         return formatted;

config/src/main/java/com/typesafe/config/ConfigResolveOptions.java

@@ -83,7 +83,7 @@ public final class ConfigResolveOptions {
      * default, unresolved substitutions are an error. If unresolved
      * substitutions are allowed, then a future attempt to use the unresolved
      * value may fail, but {@link Config#resolve(ConfigResolveOptions)} itself
-     * will now throw.
+     * will not throw.
      * 
      * @param value
      *            true to silently ignore unresolved substitutions.

config/src/main/java/com/typesafe/config/impl/Parser.java

@@ -1138,26 +1138,51 @@ final class Parser {
         }
     }
 
-    // the idea is to see if the string has any chars that might require the
-    // full parser to deal with.
-    private static boolean hasUnsafeChars(String s) {
-        for (int i = 0; i < s.length(); ++i) {
+    // the idea is to see if the string has any chars or features
+    // that might require the full parser to deal with.
+    private static boolean looksUnsafeForFastParser(String s) {
+        boolean lastWasDot = true; // start of path is also a "dot"
+        int len = s.length();
+        if (s.isEmpty())
+            return true;
+        if (s.charAt(0) == '.')
+            return true;
+        if (s.charAt(len - 1) == '.')
+            return true;
+
+        for (int i = 0; i < len; ++i) {
             char c = s.charAt(i);
-            if (Character.isLetter(c) || c == '.')
+            if ((c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') || c == '_') {
+                lastWasDot = false;
                 continue;
-            else
+            } else if (c == '.') {
+                if (lastWasDot)
+                    return true; // ".." means we need to throw an error
+                lastWasDot = true;
+            } else if (c == '-') {
+                if (lastWasDot)
+                    return true;
+                continue;
+            } else {
                 return true;
+            }
         }
+
+        if (lastWasDot)
+            return true;
+
         return false;
     }
 
-    private static void appendPathString(PathBuilder pb, String s) {
-        int splitAt = s.indexOf('.');
+    private static Path fastPathBuild(Path tail, String s, int end) {
+        // lastIndexOf takes last index it should look at, end - 1 not end
+        int splitAt = s.lastIndexOf('.', end - 1);
+        // this works even if splitAt is -1; then we start the substring at 0
+        Path withOneMoreElement = new Path(s.substring(splitAt + 1, end), tail);
         if (splitAt < 0) {
-            pb.appendKey(s);
+            return withOneMoreElement;
         } else {
-            pb.appendKey(s.substring(0, splitAt));
-            appendPathString(pb, s.substring(splitAt + 1));
+            return fastPathBuild(withOneMoreElement, s, splitAt);
         }
     }
 
@@ -1165,15 +1190,9 @@ final class Parser {
     // we just have something like "foo" or "foo.bar"
     private static Path speculativeFastParsePath(String path) {
         String s = ConfigImplUtil.unicodeTrim(path);
-        if (s.isEmpty())
+        if (looksUnsafeForFastParser(s))
             return null;
-        if (hasUnsafeChars(s))
-            return null;
-        if (s.startsWith(".") || s.endsWith(".") || s.contains(".."))
-            return null; // let the full parser throw the error
 
-        PathBuilder pb = new PathBuilder();
-        appendPathString(pb, s);
-        return pb.result();
+        return fastPathBuild(null, s, s.length());
     }
 }

config/src/test/scala/Profiling.scala

@@ -3,19 +3,40 @@
  */
 import com.typesafe.config.Config
 import com.typesafe.config.ConfigFactory
+import com.typesafe.config.ConfigException
+import java.util.concurrent.TimeUnit
+import scala.annotation.tailrec
+
 object Util {
-    def time(body: () => Unit, iterations: Int): Long = {
+    @tailrec
+    def timeHelper(body: () => Unit, iterations: Int, retried: Boolean): Double = {
         // warm up
-        for (i <- 1 to 20) {
+        for (i <- 1 to Math.max(20, iterations / 10)) {
             body()
         }
 
-        val start = System.currentTimeMillis()
+        val start = System.nanoTime()
         for (i <- 1 to iterations) {
             body()
         }
-        val end = System.currentTimeMillis()
-        (end - start) / iterations
+        val end = System.nanoTime()
+
+        val elapsed = end - start
+
+        val nanosInMillisecond = 1000000L
+
+        if (elapsed < (1000 * nanosInMillisecond)) {
+            System.err.println(s"Total time for $iterations was less than a second; trying with more iterations")
+            timeHelper(body, iterations * 10, true)
+        } else {
+            if (retried)
+                System.out.println(s"with $iterations we got a long enough sample (${elapsed.toDouble / nanosInMillisecond}ms)")
+            (elapsed.toDouble / iterations) / nanosInMillisecond
+        }
+    }
+
+    def time(body: () => Unit, iterations: Int): Double = {
+        timeHelper(body, iterations, false)
     }
 
     def loop(args: Seq[String], body: () => Unit) {
@@ -36,7 +57,7 @@ object FileLoad extends App {
         }
     }
 
-    val ms = Util.time(task, 100)
+    val ms = Util.time(task, 4000)
     println("file load: " + ms + "ms")
 
     Util.loop(args, task)
@@ -52,8 +73,82 @@ object Resolve extends App {
         }
     }
 
-    val ms = Util.time(task, 10000)
+    val ms = Util.time(task, 3000000)
     println("resolve: " + ms + "ms")
 
     Util.loop(args, task)
 }
+
+object GetExistingPath extends App {
+    val conf = ConfigFactory.parseString("aaaaa.bbbbb.ccccc.d=42").resolve()
+
+    def task() {
+        if (conf.getInt("aaaaa.bbbbb.ccccc.d") != 42) {
+            throw new Exception("broken get")
+        }
+    }
+
+    val ms = Util.time(task, 2000000)
+    println("GetExistingPath: " + ms + "ms")
+
+    Util.loop(args, task)
+}
+
+object GetSeveralExistingPaths extends App {
+    val conf = ConfigFactory.parseString("aaaaa { bbbbb.ccccc.d=42, qqqqq.rrrrr = 43 }, xxxxx.yyyyy.zzzzz = 44 ").resolve()
+
+    def task() {
+        if (conf.getInt("aaaaa.bbbbb.ccccc.d") != 42 ||
+            conf.getInt("aaaaa.qqqqq.rrrrr") != 43 ||
+            conf.getInt("xxxxx.yyyyy.zzzzz") != 44) {
+            throw new Exception("broken get")
+        }
+    }
+
+    val ms = Util.time(task, 5000000)
+    println("GetSeveralExistingPaths: " + ms + "ms")
+
+    Util.loop(args, task)
+}
+
+object HasPathOnMissing extends App {
+    val conf = ConfigFactory.parseString("aaaaa.bbbbb.ccccc.d=42,x=10, y=11, z=12").resolve()
+
+    def task() {
+        if (conf.hasPath("aaaaa.bbbbb.ccccc.e")) {
+            throw new Exception("we shouldn't have this path")
+        }
+    }
+
+    val ms = Util.time(task, 20000000)
+    println("HasPathOnMissing: " + ms + "ms")
+
+    Util.loop(args, task)
+}
+
+object CatchExceptionOnMissing extends App {
+    val conf = ConfigFactory.parseString("aaaaa.bbbbb.ccccc.d=42,x=10, y=11, z=12").resolve()
+
+    def anotherStackFrame(remaining: Int)(body: () => Unit): Int = {
+        if (remaining == 0) {
+            body()
+            123
+        } else {
+            42 + anotherStackFrame(remaining - 1)(body)
+        }
+    }
+
+    def task() {
+        try conf.getInt("aaaaa.bbbbb.ccccc.e")
+        catch {
+            case e: ConfigException.Missing =>
+        }
+    }
+
+    anotherStackFrame(40) { () =>
+        val ms = Util.time(task, 300000)
+        println("CatchExceptionOnMissing: " + ms + "ms")
+
+        Util.loop(args, task)
+    }
+}