Simplify HOCON data structure. #240

Arkatufus · 2020-02-24T17:36:25Z

Closes #205

Merge Hocon.Immutable into core Hocon, simplifying the API surface to:

HoconParser
HoconElement
HoconObject
HoconArray
HoconLiteral
HoconPath

# Conflicts: # src/Hocon.API.Tests/HoconAPISpec.ApproveConfiguration.approved.txt # src/Hocon.Configuration/Config.cs # src/Hocon.Configuration/HoconConfigurationFactory.cs

…ows SDK version.

src/Hocon/Builder/HoconArrayBuilder.cs

src/Hocon/HoconArray.cs

Arkatufus · 2020-02-24T18:16:28Z

src/Hocon.Configuration/Config.cs

@@ -27,7 +27,7 @@ public class Config : HoconRoot, ISerializable, IEquatable<Config>
        static Config()
        {
            EmptyValue = new HoconValue(null);
-            EmptyValue.Add(new HoconObject(EmptyValue));
+            EmptyValue.Add(new InternalHoconObject(EmptyValue));


Old classes still lives in the code, they are all changed from public to internal and all name collision are resolved by adding Internal to the start of the class name.

Arkatufus · 2020-02-24T18:18:17Z

src/Hocon/HoconParser.cs

@@ -38,9 +38,9 @@ public sealed class HoconParser
        ///     This exception is thrown when an unresolved substitution is encountered.
        ///     It also occurs when any error is encountered while tokenizing or parsing the configuration string.
        /// </exception>
-        public static HoconRoot Parse(string text, HoconIncludeCallbackAsync includeCallback = null)
+        public static HoconObject Parse(string text, HoconIncludeCallbackAsync includeCallback = null)


HoconRoot only used to transfer substitution from included hocon files, they are not publicly exposed anymore.
Parse() returns a HoconObject instance instead.

Arkatufus · 2020-02-24T18:34:18Z

src/Hocon/HoconRoot.cs

@@ -14,7 +14,7 @@ namespace Hocon
    ///     This class represents the root element in a HOCON (Human-Optimized Config Object Notation)
    ///     configuration string.
    /// </summary>
-    public class HoconRoot:IEquatable<HoconRoot>
+    internal class HoconRoot:IEquatable<HoconRoot>


HoconRoot is now internal.

Aaronontheweb · 2020-02-25T16:18:01Z

@Arkatufus looks like we have some merge conflicts here

…n irrepairable critical error.

# Conflicts: # Hocon.sln

…n in Akka

src/Hocon.Configuration/HoconConfigConverter.cs

…m running.

Aaronontheweb

I am not at all clear on what the benefits are of merging in this pull request or why it's needed, and given how close we are to releasing Akka.NET v1.4.0 I am very uncomfortable merging these changes.

Please help me understand:

What the benefit of introducing these changes are
Why this architectural change is necessary in both short and long runs
How much work is this going to be to test with Akka.NET - I believe you've already started answering that.

Aaronontheweb · 2020-02-27T01:26:16Z

src/Hocon/HoconObject.cs

+
+namespace Hocon
+{
+    public class HoconObject :


So why this class - what's the motivation for separating all of these into their new structures? Need to understand the high level point of view.

The main idea is to simplify the data passed back to the user. Rewriting/restructuring the existing data structure and making sure that it works with the hocon parser, substitution and external file include callback would take way too long to do and way too complicated, so instead of modifying what we have right now, I chose to transfer the tree structure to a more lean and immutable data structure.

All of the tree nodes are based on an abstract class called HoconElement that will give access to all of the data access API surface.

The HoconObject is modeled after the API of the standard ImmutableSortedDictionary<string, HoconElement> and inherits from HoconElement

The HoconArray is modeled after the API of the standard ImmutableList<HoconElement> and inherits from HoconElement

The HoconLiteral is just a simple immutable class with one string Value property that inherits from HoconElement.

The advantage is that, since everything is immutable, it is safe to pass them around, even making multiple reference to a single instance, without causing any concurrency problems, ie. they are inherently thread safe. The disadvantage is that, all the previously allocated objects are discarded, so the whole solution have a higher memory footprint than previously.

To build the tree structure, I opted to use the builder pattern. There are 3 builders:

HoconObjectBuilder basically have the exact same API as SortedDictionary<string, HoconElement>. You Add(key, value) items into it and then call the Build() function to create an immutable HoconObject

HoconArrayBuilder basically have the exact same API as List<HoconElement>. Same as HoconObjectBuilder, you Add() or AddRange() HoconElements and then call Build() and it'll return a new immutable HoconArray.

HoconLiteralBuilder basically have the exact same API as StringBuilder. Just add strings into it and call Build().

All 3 builders are chainable in fluent way, so you can do things like new HoconObjectBuilder(existingObjectDictionary).Add(element1).Add(element2).Build()

Aaronontheweb · 2020-02-27T01:26:46Z

src/Hocon.API.Tests/HoconAPISpec.ApproveCore.approved.txt

 {
-    public class static HoconElementExtensions
+    public class static ArrayGetterExtensions


Too generic of a class name - needs to be made specific to HOCON.

OK, I'll append Hocon in front of the names.

Sounds good

Aaronontheweb · 2020-02-27T01:30:57Z

src/Hocon.Configuration/Config.cs

-
-        protected List<HoconValue> _fallbacks { get; } = new List<HoconValue>();
-        public virtual IReadOnlyList<HoconValue> Fallbacks => _fallbacks.ToList().AsReadOnly();
+        private ConcurrentDictionary<string, HoconElement> _cache = new ConcurrentDictionary<string, HoconElement>();


We're going to need to get rid of the cache. Memory consumption increased 6x on this PR.

Before

Hocon.Tests.Performance.ConfigLookupSpec+Config_Lookup_HoconPath_10_Deep_throughput

Tests how quickly Config can perform a 10 deep path lookup using a HoconPath path
2/25/2020 5:45:12 PM

System Info

NBench=NBench, Version=2.0.1.0, Culture=neutral, PublicKeyToken=null OS=Microsoft Windows NT 6.2.9200.0 ProcessorCount=2 CLR=4.0.30319.42000,IsMono=False,MaxGcGeneration=2

NBench Settings

RunMode=Throughput, TestMode=Measurement NumberOfIterations=13, MaximumRunTime=00:00:01 Concurrent=True Tracing=True

Data

Totals

Metric Units Max Average Min StdDev

TotalCollections [Gen0] collections 138.00 138.00 138.00 0.00

TotalCollections [Gen1] collections 0.00 0.00 0.00 0.00

TotalCollections [Gen2] collections 0.00 0.00 0.00 0.00

TotalBytesAllocated bytes 1,227,752.00 1,227,752.00 1,227,752.00 0.00

[Counter] LookupOp operations 92,086.00 92,086.00 92,086.00 0.00

Per-second Totals

Metric Units / s Max / s Average / s Min / s StdDev / s

TotalCollections [Gen0] collections 140.30 134.00 127.71 3.78

TotalCollections [Gen1] collections 0.00 0.00 0.00 0.00

TotalCollections [Gen2] collections 0.00 0.00 0.00 0.00

TotalBytesAllocated bytes 1,248,193.67 1,192,135.24 1,136,214.79 33,612.93

[Counter] LookupOp operations 93,619.20 89,414.61 85,220.37 2,521.10

After

Hocon.Tests.Performance.ConfigLookupSpec+Config_Lookup_HoconPath_10_Deep_throughput

Tests how quickly Config can perform a 10 deep path lookup using a HoconPath path
2/26/2020 8:20:18 PM

System Info

NBench=NBench, Version=2.0.1.0, Culture=neutral, PublicKeyToken=null OS=Microsoft Windows NT 6.2.9200.0 ProcessorCount=2 CLR=4.0.30319.42000,IsMono=False,MaxGcGeneration=2

NBench Settings

RunMode=Throughput, TestMode=Measurement NumberOfIterations=13, MaximumRunTime=00:00:01 Concurrent=True Tracing=True

Data

Totals

Metric Units Max Average Min StdDev

TotalCollections [Gen0] collections 33.00 33.00 33.00 0.00

TotalCollections [Gen1] collections 0.00 0.00 0.00 0.00

TotalCollections [Gen2] collections 0.00 0.00 0.00 0.00

TotalBytesAllocated bytes 6,054,296.00 6,054,296.00 6,054,296.00 0.00

[Counter] LookupOp operations 209,371.00 209,371.00 209,371.00 0.00

Per-second Totals

Metric Units / s Max / s Average / s Min / s StdDev / s

TotalCollections [Gen0] collections 34.26 33.36 32.58 0.62

TotalCollections [Gen1] collections 0.00 0.00 0.00 0.00

TotalCollections [Gen2] collections 0.00 0.00 0.00 0.00

TotalBytesAllocated bytes 6,285,038.62 6,120,678.42 5,977,090.52 113,842.84

[Counter] LookupOp operations 217,350.59 211,666.65 206,701.06 3,936.94

The performance improvement is nice - but we read from HOCON values fairly infrequently, whereas they are retained in memory for the duration of the application and we need to be gentle with memory consumption considering that we will run inside environments like IOT where RAM is at a premium. If we want to add caching to improve performance in a specific area of HOCON, that should be up to the users sub-classing Config - not something Config does carte blanche.

I need to rework the perf spec so that it check the memory allocated with and without the config creation step.

…eflects the memory cost of the lookup spec

…me refactor

Arkatufus · 2020-02-27T16:30:05Z

I think this looks quite good.

Lookup performance using string path:

NBench Settings

RunMode=Throughput, TestMode=Measurement
NumberOfIterations=13, MaximumRunTime=00:00:01
Concurrent=True
Tracing=False

Data

Totals

Metric	Units	Max	Average	Min	StdDev
TotalCollections [Gen0]	collections	193.00	193.00	193.00	0.00
TotalCollections [Gen1]	collections	0.00	0.00	0.00	0.00
TotalCollections [Gen2]	collections	0.00	0.00	0.00	0.00
TotalBytesAllocated	bytes	509,808.00	509,788.92	509,560.00	68.78
[Counter] LookupOp	operations	311,202.00	311,202.00	311,202.00	0.00

Per-second Totals

Metric	Units / s	Max / s	Average / s	Min / s	StdDev / s
TotalCollections [Gen0]	collections	201.28	197.50	192.03	2.83
TotalCollections [Gen1]	collections	0.00	0.00	0.00	0.00
TotalCollections [Gen2]	collections	0.00	0.00	0.00	0.00
TotalBytesAllocated	bytes	531,687.42	521,666.08	507,006.76	7,518.38
[Counter] LookupOp	operations	324,557.85	318,452.11	309,642.67	4,565.09

Lookup performance using HoconPath path:

NBench Settings

RunMode=Throughput, TestMode=Measurement
NumberOfIterations=13, MaximumRunTime=00:00:01
Concurrent=True
Tracing=False

Data

Totals

Metric	Units	Max	Average	Min
TotalCollections [Gen0]	collections	0.00	0.00	0.00
TotalCollections [Gen1]	collections	0.00	0.00	0.00
TotalCollections [Gen2]	collections	0.00	0.00	0.00
TotalBytesAllocated	bytes	0.00	0.00	0.00
[Counter] LookupOp	operations	842,920.00	842,920.00	842,920.00

Per-second Totals

Metric	Units / s	Max / s	Average / s	Min / s	StdDev / s
TotalCollections [Gen0]	collections	0.00	0.00	0.00	0.00
TotalCollections [Gen1]	collections	0.00	0.00	0.00	0.00
TotalCollections [Gen2]	collections	0.00	0.00	0.00	0.00
TotalBytesAllocated	bytes	0.00	0.00	0.00	0.00
[Counter] LookupOp	operations	892,470.21	867,499.86	859,543.66	8,183.63

This reverts commit 9bff257.

* Revert "Change the default file names and load priority (#249)" This reverts commit 828a292. * Revert "HOCON v2.0.2 Release Notes (#248)" This reverts commit b4ad447. * Revert "Hyperion support (#247)" This reverts commit 94981ce. * Revert "Simplify HOCON data structure. (#240)" This reverts commit 9bff257.

* Revert "Change the default file names and load priority (#249)" This reverts commit 828a292. * Revert "HOCON v2.0.2 Release Notes (#248)" This reverts commit b4ad447. * Revert "Hyperion support (#247)" This reverts commit 94981ce. * Revert "Simplify HOCON data structure. (#240)" This reverts commit 9bff257. * added HOCON 2.0.3 release notes

Arkatufus added 11 commits February 20, 2020 20:56

Move Hocon.Immutable into Hocon namespace

84a3c5a

Mark old classes as internal classes

2eedda3

Convert old data type to new ones

280a826

Convert old data type to new data type

2e09c9d

Improve backward compatibility

84dd12c

Convert old Config to use the new data structure.

0b7beb2

Add caching to Config

2d5e60c

Update ApiApprover text file to reflect the new data structure.

10d2659

Merge branch 'dev' into Simplify_Data_Structure

2ea0ce5

# Conflicts: # src/Hocon.API.Tests/HoconAPISpec.ApproveConfiguration.approved.txt # src/Hocon.Configuration/Config.cs # src/Hocon.Configuration/HoconConfigurationFactory.cs

Bump Linux .NET SDK version from 2.1.500 to 2.1.504 to match the wind…

b5a687e

…ows SDK version.

force csproj LangVersion tag to 7.2

8c4a583

Arkatufus commented Feb 24, 2020

View reviewed changes

src/Hocon/Builder/HoconArrayBuilder.cs Show resolved Hide resolved

Arkatufus commented Feb 24, 2020

View reviewed changes

src/Hocon/HoconArray.cs Show resolved Hide resolved

Arkatufus commented Feb 24, 2020

View reviewed changes

Arkatufus added 4 commits February 25, 2020 23:46

Remove throw statement, throw does not make sense since this is not a…

1c095b6

…n irrepairable critical error.

Add missing getters needed for Akka compatibility

debc2e6

Merge branch 'dev' into Simplify_Data_Structure

3ad3d69

# Conflicts: # Hocon.sln

Add serialization hacks (surrogate) from Akka.NET to fix serializatio…

279978f

…n in Akka

Aaronontheweb reviewed Feb 25, 2020

View reviewed changes

src/Hocon.Configuration/HoconConfigConverter.cs Outdated Show resolved Hide resolved

Arkatufus mentioned this pull request Feb 25, 2020

Simplified hocon data structure implementation akkadotnet/akka.net#4253

Closed

3 tasks

Arkatufus added 5 commits February 26, 2020 03:47

Merge branch 'dev' into Simplify_Data_Structure

8fa5415

Remove Newtonsoft.Json nuget dependency

cdc4f1c

Defensive coding

d91901c

Adjust surface API

1deb2a7

Clean ghost Immutable projects from .sln file. Prevented CI build fro…

ac1e4ad

…m running.

Aaronontheweb requested changes Feb 27, 2020

View reviewed changes

Prepend Hocon to classes names that are too generic

f9b4b75

Arkatufus added 2 commits February 27, 2020 21:36

Remove caching from Config

f109c44

Force garbage collection so that memory allocation stat result only r…

63305ed

…eflects the memory cost of the lookup spec

Arkatufus marked this pull request as ready for review February 27, 2020 15:38

Update the approved API list to reflect the static extension class na…

9afc666

…me refactor

Aaronontheweb approved these changes Feb 27, 2020

View reviewed changes

Aaronontheweb merged commit 9bff257 into akkadotnet:dev Feb 27, 2020

Aaronontheweb added a commit to Aaronontheweb/HOCON that referenced this pull request Feb 28, 2020

Revert "Simplify HOCON data structure. (akkadotnet#240)"

e7456f9

This reverts commit 9bff257.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Simplify HOCON data structure. #240

Simplify HOCON data structure. #240

Arkatufus commented Feb 24, 2020 •

edited

Loading

Arkatufus Feb 24, 2020

Arkatufus Feb 24, 2020

Arkatufus Feb 24, 2020

Aaronontheweb commented Feb 25, 2020

Aaronontheweb left a comment

Aaronontheweb Feb 27, 2020

Arkatufus Feb 27, 2020 •

edited

Loading

Aaronontheweb Feb 27, 2020

Arkatufus Feb 27, 2020

Aaronontheweb Feb 27, 2020

Aaronontheweb Feb 27, 2020

Arkatufus Feb 27, 2020

Arkatufus commented Feb 27, 2020

Metric	Units	Max	Average	Min
TotalCollections [Gen0]	collections	138.00	138.00	138.00
TotalCollections [Gen1]	collections	0.00	0.00	0.00
TotalCollections [Gen2]	collections	0.00	0.00	0.00
TotalBytesAllocated	bytes	1,227,752.00	1,227,752.00	1,227,752.00
[Counter] LookupOp	operations	92,086.00	92,086.00	92,086.00

Metric	Units / s	Max / s	Average / s	Min / s	StdDev / s
TotalCollections [Gen0]	collections	140.30	134.00	127.71	3.78
TotalCollections [Gen1]	collections	0.00	0.00	0.00	0.00
TotalCollections [Gen2]	collections	0.00	0.00	0.00	0.00
TotalBytesAllocated	bytes	1,248,193.67	1,192,135.24	1,136,214.79	33,612.93
[Counter] LookupOp	operations	93,619.20	89,414.61	85,220.37	2,521.10

Metric	Units	Max	Average	Min
TotalCollections [Gen0]	collections	33.00	33.00	33.00
TotalCollections [Gen1]	collections	0.00	0.00	0.00
TotalCollections [Gen2]	collections	0.00	0.00	0.00
TotalBytesAllocated	bytes	6,054,296.00	6,054,296.00	6,054,296.00
[Counter] LookupOp	operations	209,371.00	209,371.00	209,371.00

Metric	Units / s	Max / s	Average / s	Min / s	StdDev / s
TotalCollections [Gen0]	collections	34.26	33.36	32.58	0.62
TotalCollections [Gen1]	collections	0.00	0.00	0.00	0.00
TotalCollections [Gen2]	collections	0.00	0.00	0.00	0.00
TotalBytesAllocated	bytes	6,285,038.62	6,120,678.42	5,977,090.52	113,842.84
[Counter] LookupOp	operations	217,350.59	211,666.65	206,701.06	3,936.94

Simplify HOCON data structure. #240

Simplify HOCON data structure. #240

Conversation

Arkatufus commented Feb 24, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Aaronontheweb commented Feb 25, 2020

Aaronontheweb left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Arkatufus Feb 27, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Before

Hocon.Tests.Performance.ConfigLookupSpec+Config_Lookup_HoconPath_10_Deep_throughput

System Info

NBench Settings

Data

Totals

Per-second Totals

After

Hocon.Tests.Performance.ConfigLookupSpec+Config_Lookup_HoconPath_10_Deep_throughput

System Info

NBench Settings

Data

Totals

Per-second Totals

Choose a reason for hiding this comment

Arkatufus commented Feb 27, 2020

NBench Settings

Data

Totals

Per-second Totals

NBench Settings

Data

Totals

Per-second Totals

Arkatufus commented Feb 24, 2020 •

edited

Loading

Arkatufus Feb 27, 2020 •

edited

Loading