Optimizations for FileMatcher.GetFiles #2572

maca88 · 2017-10-01T10:11:48Z

Here are the optimization changes that were discussed in #2392, which are divided into three commits:

Usage of Directory.EnumerateFiles and Directory.EnumerateDirectories and checking excludes with a custom wildcard matching algorithm.
Adding directory caching passed by LazyItemEvaluator
Traversing subdirectories in parallel by using Parallel.ForEach

For the parallel change I did some modifications in order to prevent spawning too much threads. With the old modification the traversing created up to 9 threads on my computer, with the new one, it creates up to 4 threads. I found this to be a good balance between performance and threads being used, at least on my computer (Intel I7 860).

This test was used on NHibernate.Test project and Roslyn root folder. For NHibernate also the evaluation was tested by using the Project class.

Latest master:

NH:
File spec: **/*, Call time: 2650ms, Returned files: 14
File spec: **/*.cs, Call time: 1556ms, Returned files: 3509
File spec: **/*.resx, Call time: 1548ms, Returned files: 0
File spec: **\*.hbm.xml, Call time: 258ms, Returned files: 827
File spec: **\*.jpg, Call time: 246ms, Returned files: 1
Total time: 6261ms

NHibernate.Test Project Evaluation: Total time: 7391ms

Roslyn:
File spec: **/*, Call time: 5572ms, Returned files: 5485
File spec: **/*.cs, Call time: 3398ms, Returned files: 8631
File spec: **/*.resx, Call time: 3187ms, Returned files: 70
File spec: **\*.hbm.xml, Call time: 516ms, Returned files: 0
File spec: **\*.jpg, Call time: 524ms, Returned files: 0
Total time: 13199ms

This PR:

NH:
File spec: **/*, Call time: 172ms, Returned files: 14
File spec: **/*.cs, Call time: 114ms, Returned files: 3509
File spec: **/*.resx, Call time: 63ms, Returned files: 0
File spec: **\*.hbm.xml, Call time: 68ms, Returned files: 827
File spec: **\*.jpg, Call time: 64ms, Returned files: 1
Total time: 483ms

NHibernate.Test Project Evaluation: Total time: 2097ms

Roslyn:
File spec: **/*, Call time: 748ms, Returned files: 5485
File spec: **/*.cs, Call time: 373ms, Returned files: 8631
File spec: **/*.resx, Call time: 186ms, Returned files: 70
File spec: **\*.hbm.xml, Call time: 170ms, Returned files: 0
File spec: **\*.jpg, Call time: 177ms, Returned files: 0
Total time: 1656ms

All tests were ran in Release configuration on Intel I7 860 processor and the average of 5 runs was taken.

dnfclas · 2017-10-01T10:11:53Z

@maca88,
Thanks for your contribution.
To ensure that the project team has proper rights to use your work, please complete the Contribution License Agreement at https://cla2.dotnetfoundation.org.

It will cover your contributions to all .NET Foundation-managed open source projects.
Thanks,
.NET Foundation Pull Request Bot

dnfclas · 2017-10-01T10:19:04Z

@maca88, thanks for signing the contribution license agreement. We will now validate the agreement and then the pull request.

Thanks, .NET Foundation Pull Request Bot

radical · 2017-10-01T16:35:08Z

src/Shared/FileMatcher.cs

+                    if (type == FileSystemEntity.Directories)
+                    {
+                        return entriesCache.GetOrAdd($"{path};{pattern ?? "*"}",
+                            s => new CachedEnumerable<string>(s_defaultGetFileSystemEntries(type, path, pattern,


Given that s_defaultGetFileSystemEntries gets the whole array of entries in one go (https://github.com/Microsoft/msbuild/blob/master/src/Shared/FileMatcher.cs#L256), wouldn't it be better to use something like ImmutableArray<T>, initialize with the full array and avoid the locks and contention in CachedEnumerable?

The main idea of CachedEnumerable was to save one iteration through the IEnumerable<string> retrived by Directory.EnumerateDirectories. After some testing I realized that there are no noticable differences by initialize the IEnumerable with ToImmutableArray method prior returning. I will remove the CachedEnumerable and cache an ImmutableArray<string> instead.

Oh, but you are switching that to using EnumerateDirectories later so this doesn't matter.

So, if you are reading all of it here itself then we can switch back to using Directory.GetDirectories and to string[] from IEnumerable<string>.

Also, it would be good to use git rebase -i to rework the commits whenever you want to change or fix something in them. That way they look like clean commits and makes it easier to review.

Just thinking aloud - Directory.EnumerateDirectories should be helping the amount of memory being used at least, in case of lot of directories. Didn't it?

cdmihai · 2017-10-03T20:19:27Z

Looking into the tests.

maca88 · 2017-10-03T20:49:30Z

The problem is with the usage of Path.GetFileName that doesn't treat a backslash as a director separator on Unix, so Path.GetFileName("Folder\MyFile.cs") will return "Folder\MyFile.cs" instead of "MyFile.cs". ~~My plan is to add a custom GetFileName that will work the same on Windows and Unix~~. Also, I suspect that Path.GetDirectoryName here will not work in all cases, I will do some investigation tomorrow.

cdmihai · 2017-10-03T23:34:26Z

I think the main issue here is the testing mock file system returns paths with backslashes in them, regardless of the OS. I guess the GetAccessibleFileSystemEntries method should always return paths that work well with Path.GetDirectoryName / Path.GetFileName.

I'll try and make the mock normalize its slashes.

maca88 · 2017-10-04T17:32:28Z

Fixed directory matching and flattened the AggregateException in case the current settings for the parallel traversing will be altered to have nested Parallel.ForEach calls. With the current settings only one Parallel.ForEach is used per GetFiles call, as MaxTasksPerIteration is set to the same value as MaxTasks.

maca88 · 2017-10-05T07:15:36Z

There seems to be an inconsistent behavior with excludes and case sensibility. In the current master, when an exclude is matched via regex the matching is case-insensitive and when is matched with the searchesToExcludeInSubdirs dictionary, is case-sensitive, as the dictionary does not use a case-insensitive comparer. For example, if we modify this line to @"**\bIN\**\*.cs" the test still succeeds, but if @"src\Common\**" is changed to @"src\CoMMon\**" the test fails. What should be the correct behavior? If case-insensitive is the answer, then I will modify the pattern matching algorithm to support it. Also, with this PR, an exclude like @"src\Co??on\**" will exclude files from "src\Common", but in the in current master, it won't. Is this behavior desired, or should we change it in order to behave the same as in the current master?

cdmihai · 2017-10-05T22:29:56Z

@maca88

Regarding case sensitivity, I checked the version that shipped with VS 2015 Update 3 (before we did any improvements to file globbing). Back then the exclude specs were not sent to GetFiles, the disk got walked once for include, and once for each exclude, and the include file set was obtained by subtracting the excluded files from the included ones. The only matching that happened was the Regex one, which was case insensitive. So to keep with previous behaviour, both matches need to be case insensitive.

Regarding the double question mark match, master seems to work, it excludes src/common/a.cs with src/co??on/**:

e:\projects\tests\projects\play>tree /f /a

E:.
|   build.proj
|   MaxPathFinder.exe
|
+---Common
|       a.cs
|
\---src
    |   c.cs
    |
    +---Common
    |       a.cs
    |
    \---Comon
            b.cs


e:\projects\tests\projects\play>type build.proj
<Project xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
    <ItemGroup>
        <Compile Include="**\*.cs" Exclude="src\Co??on\**"/>
        <Compile2 Include="**\*.cs" Exclude="**\Co??on\**"/>
        <Compile3 Include="**\*.cs" Exclude="Co??on\**"/>
    </ItemGroup>

    <Target Name="Build">
        <Message Text=" Compile: @(Compile)"/>
        <Message Text="Compile2: @(Compile2)"/>
        <Message Text="Compile3: @(Compile3)"/>
    </Target>
</Project>

e:\projects\tests\projects\play>e:\projects\msbuild\bin\Bootstrap\MSBuild\15.0\Bin\MSBuild.exe
Microsoft (R) Build Engine version 15.5.98.54482 for .NET Framework
Copyright (C) Microsoft Corporation. All rights reserved.

Build started 10/5/2017 3:25:12 PM.
Project "e:\projects\tests\projects\play\build.proj" on node 1 (default targets).
Build:
   Compile: Common\a.cs;src\c.cs;src\Comon\b.cs
  Compile2: src\c.cs;src\Comon\b.cs
  Compile3: src\c.cs;src\Common\a.cs;src\Comon\b.cs
Done Building Project "e:\projects\tests\projects\play\build.proj" (default targets).


Build succeeded.
    0 Warning(s)
    0 Error(s)

Time Elapsed 00:00:00.36

davkean · 2017-10-05T22:32:57Z

src/Shared/FileMatcher.cs

+            //    extensions that start with the same three characters e.g. "*.htm" would match both "file.htm" and "file.html"
+            // 3) if the ? wildcard is to the left of a period, it matches files with shorter name e.g. ???.txt would match
+            //    foo.txt, fo.txt and also f.txt
+            string extensionPart = Path.GetExtension(searchPattern);


Please don't allocate here unless needed, make sure this occurs only when if the search for "?." succeeds.

davkean · 2017-10-05T22:46:31Z

src/Shared/FileMatcher.cs

            if (Directory.Exists(path))
            {
                try
                {
-                    entries = Directory.GetFileSystemEntries(path, pattern);
+                    return ShouldEnforceMatching(pattern)
+                        ? Directory.EnumerateFileSystemEntries(path, pattern).Where(o => IsMatch(Path.GetFileName(o), pattern))


You need to think about exception semantics, no longer are exceptions isolated here - they can be thrown when the "enumerable" is iterated.

Ouch, I completely missed that. I will wrap the enumerable into a "safe" enumerable that will omit those two exceptions, in order to preserve the same behavior, prior this change.

no longer are exceptions isolated here

Sadly, this is not actually true, when EnumerateFileSystemEntries/EnumerateFiles/EnumerateDirectories methods are called, they can also throw because the file enumerator is initialized by finding the first file/directory and if an error occurs, the exception is immediately thrown. So we have to handle exceptions when one of those three methods is called and on the MoveNext call.

davkean · 2017-10-05T22:46:44Z

src/Shared/FileMatcher.cs

@@ -372,7 +412,7 @@ GetFileSystemEntries getFileSystemEntries
                    else
                    {
                        // getFileSystemEntries(...) returns an empty array if longPath doesn't exist.
-                        string[] entries = getFileSystemEntries(FileSystemEntity.FilesAndDirectories, longPath, parts[i], null, false);
+                        string[] entries = getFileSystemEntries(FileSystemEntity.FilesAndDirectories, longPath, parts[i], null, false).ToArray();


This can throw.

davkean · 2017-10-05T22:48:27Z

src/Shared/FileMatcher.cs

        )
        {
-            for (int i = 0; i < paths.Length; i++)
+            foreach (string path in paths)


This can throw.

davkean · 2017-10-05T22:48:36Z

src/Shared/FileMatcher.cs

            string projectDirectory
        )
        {
            bool directoryLastCharIsSeparator = IsDirectorySeparator(projectDirectory[projectDirectory.Length - 1]);
-            for (int i = 0; i < paths.Length; i++)
+            foreach (string path in paths)


This can throw.

All of these will be caught via the aggregate exception in GetFiles right?

In reply to: 143076202 [](ancestors = 143076202)

Actually, they will be, but this is not correct as we don't want to return "empty" result just because one of the subdirectories, failed to get its files. The catching of the AggreateException is supposed to do the same as the other catch, when the exception is thrown inside a Parallel.ForEach, which always throws an AggreateException containing the actual exception. The iteration over paths should not throw for SecurityException and UnauthorizedAccessException in order to preserve the old behavior, I will correct this.

Edit:
Fixed

davkean · 2017-10-05T22:49:48Z

src/Shared/FileMatcher.cs

@@ -1905,21 +2123,42 @@ DirectoryExists directoryExists
             * This is because it's cheaper to add items to an IList and this code


Comment is outdated.

davkean · 2017-10-05T22:50:54Z

src/Shared/FileMatcher.cs

+                // Flatten to get exceptions than are thrown inside a nested Parallel.ForEach
+                if (ex.Flatten().InnerExceptions.All(ExceptionHandling.IsIoRelatedException))
+                {
+                    return CreateArrayWithSingleItemIfNotExcluded(filespecUnescaped, excludeSpecsUnescaped);


Oh, is this to handle the exceptions? If so, why we do we still need the catch blocks above?

Check the comment above for catching the AggregateException.

davkean · 2017-10-05T22:51:57Z

src/Shared/FileMatcher.cs

+                // Set to use only half processors when we have 4 or more of them, in order to not be too aggresive
+                // By setting MaxTasksPerIteration to the maximum amount of tasks, which means that only one
+                // Parallel.ForEach will run at once, we get a stable number of threads being created.
+                var maxTasks = Math.Max(1, Environment.ProcessorCount / 2);


@lifengl What do you think about this when it runs inside VS?

That seems to be fine to me. In dev 14 time, during the solution loading time, we use about 2.2 cores inside VS, and design time build uses about 1 core in the background. However, in .net core project, after we disabled the async tree loading, and with very long time in the evaluation, it falls back to almost one core, especially during the project evaluation phase. So this change will make the evaluation more efficient during that phase.

In the project editing phase, or project reloading time, we may have multiple evaluation in the background. Ideally, if we can control that based on the priority of the evaluation (foreground or background), it will be better. But it looks like this setting is not too aggressive.

davkean

This is really great contribution, thanks @maca88. I'm not clear on the exception semantics of the change - so I'd like that clarified - what happens when an exception is thrown as we iterate?

davkean · 2017-10-05T22:54:40Z

src/Build/Evaluation/LazyItemEvaluator.LazyItemOperation.cs

+            /// <summary>
+            /// Cache used for caching IO operation results
+            /// </summary>
+            protected ConcurrentDictionary<string, IEnumerable<string>> EntriesCache => _lazyEvaluator._entriesCache;


@cdmihai Help me understand the lifetime of this cache? Just for single Include or for all item evaluations?

it spans the entire item evaluation phase, so covers all items outside of targets

In reply to: 143077078 [](ancestors = 143077078)

maca88 · 2017-10-06T15:26:12Z

@cdmihai

So to keep with previous behaviour, both matches need to be case insensitive.

I will modify the algorithm to support case-insensitive matching as fast as possible, correct the initialization of searchesToExcludeInSubdirs dictionary to use a case-insensitive comparer and add tests for complex patterns using the real file system.

Regarding the double question mark match, master seems to work

My bad, I didn't noticed that the error was due to the mock file system, which doesn't support complex patterns.

maca88 · 2017-10-06T16:02:00Z

@davkean

The exception handling should now work like before, with two exceptions:

If an exception is thrown inside a Parallel.ForEach and it is not handled by ExceptionHandling.IsIoRelatedException, then an AggregateException will be thrown instead of the actual exception/s.
By replacing the Directory.GetXXX with Directory.EnumerateXXX we now may get one or more files/directories before the first Security/UnauthorizedAccess exception is thrown, in other words, if the third file is not accessible, with Directory.EnumerateXXX we would get the first two, but with Directory.GetXXX we wouldn't get any.

Edit:

In order to avoid the second point, I've changed the IEnumerable to an ImmutableArray, so that the files/directories will be iterated prior returning. I ran the same tests as in the first post to see if the performance dropped, but it didn't, the results were more or less the same.

lifengl · 2017-10-06T19:33:06Z

src/Shared/FileMatcher.cs

@@ -372,7 +478,7 @@ GetFileSystemEntries getFileSystemEntries
                    else
                    {
                        // getFileSystemEntries(...) returns an empty array if longPath doesn't exist.
-                        string[] entries = getFileSystemEntries(FileSystemEntity.FilesAndDirectories, longPath, parts[i], null, false);
+                        string[] entries = getFileSystemEntries(FileSystemEntity.FilesAndDirectories, longPath, parts[i], null, false).ToArray();


Minor: ToList() is usually slightly faster, (they are almost same to copy data to a buffer, but ToArray() often copies one more time to fit the exact size.

With the recent change, the ToArray is not needed anymore.

lifengl · 2017-10-06T19:41:40Z

That is really nice work, @maca88. I saw so many traces that the slow globbing code consumed 10 seconds or even 30 seconds in some projects. This change will make a huge difference.

maca88 · 2017-10-06T20:13:21Z

src/Build.UnitTests/Microsoft.Build.Engine.UnitTests.csproj

@@ -249,6 +249,7 @@
    </ProjectReference>
  </ItemGroup>
  <ItemGroup Condition="'$(NetCoreBuild)' != 'true'">
+    <Reference Include="System.Collections.Immutable, Version=1.2.1.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a" />


I've had to add this reference in order to build the project. Does anyone know why?

I think it's because ImmutableArray got exposed to the tests (FileMatcher_Tests, via the new cache). And then FileMatcher_Tests is included in two test dlls.

I understand that the two test dlls need the dependency, what I don't understand is that adding the dependency to the project.json don't work for Engine.UnitTests, but it works for the Tasks.UnitTests project. Its seems like that one of the Engine.UnitTests dependency needs a lower version of the Immutable dll, but from the logs it is not clear which one.

lifengl · 2017-10-06T20:29:04Z

src/Shared/FileMatcher.cs

+            // Add all matched files at once to reduce thread contention
+            if (files != null && files.Any())
+            {
+                listOfFiles.PushRange(files.ToArray());


Instead of copying the list to an array, and then create a link list node for each of them (inside the stack code), I think it can be Stack<List>(), so you just push the non-empty list there. In the code consuming it, it will be:
stack.SelectMany(list => list).xxxx..., so we can prevent the extra allocation, in case the list is big.

AndyGerlicher · 2017-10-06T23:49:29Z

@dotnet-bot test Windows_NT Build for Full please

davkean · 2017-10-07T01:18:56Z

src/Shared/FileMatcher.cs

+            // Lock only when we may be dealing with multiple threads
+            if (taskOptions.MaxTasks > 1 && taskOptions.MaxTasksPerIteration > 1)
+            {
+                lock (s_directoryLock)


Can you explain what you are protecting against?

.. and rename the variable to reflect that. (s_taskOptionsLock?)

This lock prevents multiple threads that were created from different Parallel.ForEach calls from modifying the dop variable in order to assure, that we are not going to use more tasks that have been specified in TaskOptions. We have to consider that this method is recursive, which means that we could have one Parallel.ForEach that may call another Parallel.ForEach inside its iteration. I've added an additional condition for locking, as with the current settings the lock is not needed. To demonstrate how this works, I've created a simulation, where TaskOptions is configured to have up to 4 Parallel.ForEach running concurrently. By commenting the lock statement, we can see that after several runs, we may get 5 Parallel.ForEach running concurrently.

Can we just lock the taskoptions instead of creating another object?

Good idea! By doing so, we will have the locking per call, which is more correct than having it static, as the purpose of the locking is just to modify the TaskOptions.

maca88 · 2017-10-07T18:31:34Z

@cdmihai

I have corrected the case sensitivity behavior, so that excludes are now matched case-insensitive. I've added a unit test for each correction, which would fail on the current master. The overall performance after adding the case-insensitive matching remains more or less the same as before.

Edit:

There are some specific cases when an include or exclude pattern won't be matched case-insensitive on Unix. By quick checking the version that shipped with VS 2015 Update 3, seems that it has the same problems. Should we corrrect them or should we leave as it is, for backward compatibility?

AndyGerlicher · 2017-10-08T03:39:30Z

@dotnet-bot test this please

cdmihai · 2017-10-09T17:57:41Z

@maca88

There are some specific cases when an include or exclude pattern won't be matched case-insensitive on Unix. By quick checking the version that shipped with VS 2015 Update 3, seems that it has the same problems. Should we correct them or should we leave as it is, for backward compatibility?

Leave them as is, to reduce scope creep :) I'll open an issue linking to your test once this PR gets in.

cdmihai · 2017-10-10T00:31:29Z

FYI, we're planning to merge this in tomorrow morning (10am PDT). @maca88 can you please squash your changes if you get the chance? (If not I'll do it, I just need to make sure you are preserved as commit author)

davkean · 2017-10-10T07:32:14Z

@maca88 can you please contact me offline davkean@microsoft.com?

Optimizations: - Replaced Directory methods GetFiles and GetDirectories with EnumerateFiles and EnumerateDirectories - Added a custom wildcard pattern matching algorithm for matching directory and file names - Added directory caching - Added parallelization of subdirectories

maca88 · 2017-10-10T15:15:15Z

@cdmihai
I've squashed my changes into one commit on top of the mock file system correction.

AndyGerlicher · 2017-10-10T15:39:46Z

@maca88 this is a very substantial contribution, probably the biggest we've had on MSBuild outside Microsoft. Thank you so much for seeing this through and addressing the numerous comments and iterations! This will definitely be a big part of the perf gains we'll get out of 15.5!

JesperTreetop · 2017-10-11T08:26:56Z

My plan is to add a custom GetFileName that will work the same on Windows and Unix.

I just came here to ooh and aah at the performance improvements but I really hope the quoted part (in an earlier comment by @maca88) wasn't implemented because there are definitely Unix file systems where you are allowed to have backslashes in filenames, and trying to turn them into slashes could be a logic error at best and point to completely the wrong file at worst. There's security consequences to treating Unix and Windows paths the same way (or even just not like themselves).

maca88 · 2017-10-11T18:05:34Z

@JesperTreetop the quoted part wasn't implemented, I updated the missleading part of the comment in order to avoid confusion.

dnfclas added the cla-required label Oct 1, 2017

dnfclas added cla-signed and removed cla-required labels Oct 1, 2017

radical reviewed Oct 1, 2017

View reviewed changes

maca88 force-pushed the performance/getFiles branch from 5a702a2 to e5ba128 Compare October 4, 2017 18:17

cdmihai requested review from dsplaisted and davkean October 5, 2017 00:58

davkean reviewed Oct 5, 2017

View reviewed changes

davkean requested changes Oct 5, 2017

View reviewed changes

davkean reviewed Oct 5, 2017

View reviewed changes

lifengl reviewed Oct 6, 2017

View reviewed changes

maca88 commented Oct 6, 2017

View reviewed changes

lifengl reviewed Oct 6, 2017

View reviewed changes

davkean reviewed Oct 7, 2017

View reviewed changes

davkean approved these changes Oct 7, 2017

View reviewed changes

maca88 force-pushed the performance/getFiles branch 5 times, most recently from edbe8f5 to d06a0c0 Compare October 8, 2017 03:34

maca88 force-pushed the performance/getFiles branch from d06a0c0 to 59a1395 Compare October 8, 2017 03:50

cdmihai approved these changes Oct 9, 2017

View reviewed changes

davkean added the performance label Oct 10, 2017

cdmihai and others added 2 commits October 10, 2017 16:59

Mock File System should return paths with OS specific slashes

0c3e87c

maca88 force-pushed the performance/getFiles branch from 59a1395 to 246a185 Compare October 10, 2017 15:14

AndyGerlicher approved these changes Oct 10, 2017

View reviewed changes

AndyGerlicher merged commit aebbb44 into dotnet:master Oct 10, 2017

davkean mentioned this pull request Oct 10, 2017

For large projects with globs, MSBuild takes up to 30% - 50% of a build searching the disk #2392

Closed

maca88 mentioned this pull request Oct 11, 2017

Upgrade to MSBuild 15.5 when released maca88/AsyncGenerator#58

Closed

fredericDelaporte mentioned this pull request Oct 16, 2017

#750 - AliasToBean failure, test case and fix nhibernate/nhibernate-core#1380

Merged

Forgind mentioned this pull request Feb 18, 2021

Optimize glob evaluation #6151

Merged

		@@ -1905,21 +2123,42 @@ DirectoryExists directoryExists
		* This is because it's cheaper to add items to an IList and this code

Optimizations for FileMatcher.GetFiles #2572

Optimizations for FileMatcher.GetFiles #2572

Conversation

maca88 commented Oct 1, 2017 • edited

dnfclas commented Oct 1, 2017

dnfclas commented Oct 1, 2017

Choose a reason for hiding this comment

maca88 Oct 1, 2017 • edited

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

cdmihai commented Oct 3, 2017

maca88 commented Oct 3, 2017 • edited

cdmihai commented Oct 3, 2017 • edited

maca88 commented Oct 4, 2017 • edited

maca88 commented Oct 5, 2017

cdmihai commented Oct 5, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maca88 Oct 6, 2017 • edited

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maca88 Oct 6, 2017 • edited

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davkean left a comment

Choose a reason for hiding this comment

davkean Oct 5, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maca88 commented Oct 6, 2017

maca88 commented Oct 6, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lifengl commented Oct 6, 2017

Choose a reason for hiding this comment

cdmihai Oct 6, 2017 • edited

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

AndyGerlicher commented Oct 6, 2017

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

davkean Oct 7, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maca88 commented Oct 7, 2017 • edited

AndyGerlicher commented Oct 8, 2017

cdmihai commented Oct 9, 2017

cdmihai commented Oct 10, 2017

davkean commented Oct 10, 2017

maca88 commented Oct 10, 2017

AndyGerlicher commented Oct 10, 2017

JesperTreetop commented Oct 11, 2017 • edited

maca88 commented Oct 11, 2017

maca88 commented Oct 1, 2017 •

edited

maca88 Oct 1, 2017 •

edited

maca88 commented Oct 3, 2017 •

edited

cdmihai commented Oct 3, 2017 •

edited

maca88 commented Oct 4, 2017 •

edited

cdmihai commented Oct 5, 2017 •

edited

maca88 Oct 6, 2017 •

edited

maca88 Oct 6, 2017 •

edited

davkean Oct 5, 2017 •

edited

maca88 commented Oct 6, 2017 •

edited

cdmihai Oct 6, 2017 •

edited

davkean Oct 7, 2017 •

edited

maca88 commented Oct 7, 2017 •

edited

JesperTreetop commented Oct 11, 2017 •

edited