Add initial checkpointing to the nullable walker. by 333fred · Pull Request #35850 · dotnet/roslyn

333fred · 2019-05-21T20:37:32Z

This adds the initial code for checkpointing the nullable walker, and defines the structure of a checkpoint. This PR does not do anything with the checkpoints yet: MemberSemanticModel gets the checkpoints, and then just throws them away without saving them. The intention is that it will save the checkpoints and use them in the speculative model in a follow up PR. @dotnet/roslyn-compiler @AlekseyTs @gafter for review.

333fred · 2019-05-21T20:39:12Z

src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs

            return operandType;
        }

+        private void CheckpointWalkerThroughConversionGroup(BoundConversion conversionOpt, BoundExpression convertedNode)


I chose to add this as a separate method instead of putting it in RemoveConversion directly, as RemoveConversion is often used without then visiting the underlying expression. There are tradeoffs to both though, so there's preference on adding it to RemoveConversion, controlled by a default parameter, I can do that as well. #ByDesign

333fred · 2019-05-21T20:40:20Z

src/Compilers/CSharp/Portable/Compilation/MemberSemanticModel.SpeculativeMemberSemanticModel.cs

                // Currently, it will return incorrect info because it's just running analysis on the speculated
                // part.
-                return NullableWalker.AnalyzeAndRewrite(Compilation, MemberSymbol as MethodSymbol, boundRoot, binder, diagnostics);
+                return NullableWalker.AnalyzeAndRewrite(Compilation, MemberSymbol as MethodSymbol, boundRoot, binder, diagnostics, checkpointMapOpt: null);


In a follow-up PR I will be asserting that a checkpoint map isn't passed to this version of the semantic model. #ByDesign

cston · 2019-05-21T20:47:36Z

src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.Structs.cs

+                    var pooledDict = PooledDictionary<K, V>.GetInstance();
+                    foreach (var (key, value) in dictOpt)
+                    {
+                        pooledDict[key] = value;


pooledDict[key] = value [](start = 24, length = 23)

Please use .Add(key, value) to ensure there are no duplicates. #Resolved

cston · 2019-05-21T20:49:31Z

src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.Structs.cs

+                ArrayBuilder<(BoundReturnStatement, TypeWithAnnotations)> returnTypesOpt = null;
+                if (!_returnTypesOpt.IsDefault)
+                {
+                    returnTypesOpt = ArrayBuilder<(BoundReturnStatement, TypeWithAnnotations)>.GetInstance(_returnTypesOpt.Length);


ArrayBuilder [](start = 37, length = 12)

new ArrayBuilder<...>() since it doesn't look like this will be freed. #Resolved

cston · 2019-05-21T20:51:58Z

src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs

            Binder binder,
-            DiagnosticBag diagnostics)
+            DiagnosticBag diagnostics,
+            Dictionary<BoundNode, Checkpoint> checkpointMapOpt)


Dictionary<BoundNode, Checkpoint> checkpointMapOpt [](start = 12, length = 50)

Could this be returned as an ImmutableDictionary<,>? #Resolved

I expect this method to be used both by the speculative semantic path and non-speculative path, and the speculative path won't need to actually do the checkpointing. I could change this to be a bool flag about whether to do the checkpointing and return null in that case if you'd prefer.

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

Changing to bool createMap, out ImmutableDictionary<,> map sounds reasonable.

In reply to: 286219930 [](ancestors = 286219930,286218983)

cston · 2019-05-21T20:56:37Z

src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.Structs.cs

+                    walker._useMethodSignatureParameterTypes,
+                    walker._symbol,
+                    walker._returnTypesOpt?.ToImmutable() ?? default,
+                    walker._methodGroupReceiverMapOpt?.ToImmutableDictionary(),


walker._methodGroupReceiverMapOpt?.ToImmutableDictionary(), [](start = 20, length = 59)

Can we share this ImmutableDictionary<,> with all checkpoints, until the original map changes? Same question for other immutable snapshots here. #Resolved

If we're creating checkpoints at each node, then perhaps the actual NullableWalker fields should be immutable objects so the checkpoints can simply copy the fields by reference.

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

This will force us to create allocations for these new objects in the general straight-line case, which is a tradeoff we can make but I want to be sure we're deliberate about choosing to do so.

In reply to: 286227877 [](ancestors = 286227877,286220737)

cston · 2019-05-21T21:23:54Z

src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.Structs.cs

+        /// Contains a checkpoint for the NullableWalker at any given point of execution, used for restoring the walker to
+        /// a specific point for speculatively analyzing a piece of code that does not appear in the original tree.
+        /// </summary>
+        internal readonly struct Checkpoint


Checkpoint [](start = 33, length = 10)

Should this be named "Snapshot" perhaps? "Checkpoint" sounds like a point in time where a number of checks are performed rather than capturing state. #Resolved

I don't know if it's the gamer in me, but in video games a "checkpoint" is the last state you go back to if you fail the level. I can use a different word if you really want, but checkpoint is correct for this as well.

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

…sible, and add a caching strategy to the snapshotter to reduce garbage creation for every node. Addressed minor PR feedback items as well.

AlekseyTs · 2019-05-29T18:24:02Z

src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.Snapshotter.cs

+        /// Contains a checkpoint for the NullableWalker at any given point of execution, used for restoring the walker to
+        /// a specific point for speculatively analyzing a piece of code that does not appear in the original tree.
+        /// </summary>
+        internal readonly struct Snapshot


Snapshot [](start = 33, length = 8)

It feels like this structure can be significantly trimmed as we discussed offline. #Closed

AlekseyTs · 2019-05-29T18:55:46Z

src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.Snapshotter.cs

+                    binder.Conversions,
+                    this._variableState,
+                    returnTypesOpt: default,
+                    analyzedNullabilityMap,


analyzedNullabilityMap [](start = 20, length = 22)

Is the walker going to add to the map or to look for something in it? #Closed

It will add to the map. This is how nullability info on the speculative code will be conveyed to the speculative model.

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

AlekseyTs · 2019-05-29T20:21:52Z

src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.Snapshotter.cs

+                    {
+                        // The first slot always does not exist for the purposes of caching and can be ignored
+                        // in the exit condition check
+                        if (i != 0 && !walkerVariableBySlot[i].Exists)


i != 0 [](start = 28, length = 6)

Consider moving this logic out of the loop and starting at i = 1. #Closed

I'm ripping out this caching logic entirely, so I will close these comments as won't fix as they're no longer relevant.

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

AlekseyTs · 2019-05-29T20:31:37Z

src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.Snapshotter.cs

+                        // in the exit condition check
+                        if (i != 0 && !walkerVariableBySlot[i].Exists)
+                        {
+                            break;


break; [](start = 28, length = 6)

Can we use nextVariableSlot instead to figure out required capacity of the builder and to know how many items to copy with AddRange? #Closed

AlekseyTs · 2019-05-29T20:37:48Z

src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.Snapshotter.cs

+                    int i;
+                    for (i = 0; i < _lastVariableBySlot.Length; i++)
+                    {
+                        Debug.Assert(compareIdentifiers(_lastVariableBySlot[i], _walkerToSnapshot.variableBySlot[i]));


Debug.Assert(compareIdentifiers(_lastVariableBySlot[i], _walkerToSnapshot.variableBySlot[i])); [](start = 24, length = 94)

It looks like this code can be simplified if we assert that _lastVariableBySlot.Length == nextVariableSlot #Closed

Assuming that we handle always empty 0's element outside of the loop.

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

AlekseyTs · 2019-05-29T20:41:21Z

src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.Snapshotter.cs

+                    Debug.Assert(_lastVariableSlot.Count == _walkerToSnapshot._variableSlot.Count);
+                    foreach ((VariableIdentifier key, int value) in _lastVariableSlot)
+                    {
+                        if (!key.Exists) continue;


continue; [](start = 41, length = 9)

Is this code reachable? #Closed

Yes, slot 0 never exists. However, this code is now gone.

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

AlekseyTs · 2019-05-29T20:55:13Z

src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.Snapshotter.cs

+#endif
+            }
+
+            internal void SnapshotWalker(BoundNode node)


SnapshotWalker [](start = 26, length = 14)

Is this function supposed to take a snapshot? The name is very confusing then. Consider renaming. #Closed

AlekseyTs · 2019-05-29T20:55:49Z

src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.Snapshotter.cs

+                _lastConditionalAccessSlot = lastConditionalAccessSlot;
+            }
+
+            internal static Snapshot SnapshotWalker(NullableWalker walker) =>


SnapshotWalker [](start = 37, length = 14)

The name feels very confusing. #Closed

AlekseyTs · 2019-05-29T20:58:36Z

src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.Snapshotter.cs

+
+            internal static Snapshot SnapshotWalker(NullableWalker walker) =>
+                new Snapshot(
+                    walker._snapshotter.GetVariableState(walker.State),


walker.State [](start = 57, length = 12)

Wouldn't we want to clone the state? #Closed

AlekseyTs · 2019-05-29T21:06:27Z

src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.Snapshotter.cs

+        /// taking a snapshot of the NullableWalker in order to be as efficient
+        /// as possible when creating many snapshots.
+        /// </summary>
+        private sealed class Snapshotter


Snapshotter [](start = 29, length = 11)

It feels like we should test this machinery one way or another.

AlekseyTs · 2019-05-29T21:25:02Z

src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs

        /// Return statements and the result types from analyzing the returned expressions. Used when inferring lambda return type in MethodTypeInferrer.
        /// </summary>
-        private readonly ArrayBuilder<(BoundReturnStatement, TypeWithAnnotations)> _returnTypesOpt;
+        private ImmutableArray<(BoundReturnStatement, TypeWithAnnotations)> _returnTypesOpt;


private ImmutableArray<(BoundReturnStatement, TypeWithAnnotations)> _returnTypesOpt; [](start = 8, length = 84)

I do not believe there is any advantage to change the type of this field to immutable array. #Closed

AlekseyTs · 2019-05-29T21:26:03Z

src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs

        /// (Could be replaced by _analyzedNullabilityMapOpt if that map is always available.)
        /// </summary>
-        private PooledDictionary<BoundExpression, TypeWithState> _methodGroupReceiverMapOpt;
+        private ImmutableDictionary<BoundExpression, TypeWithState> _methodGroupReceiverMapOpt;


private ImmutableDictionary<BoundExpression, TypeWithState> _methodGroupReceiverMapOpt; [](start = 8, length = 87)

I do not believe there is any advantage in changing the type of this field. #Closed

AlekseyTs · 2019-05-29T21:26:34Z

src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs

        /// Placeholder locals, e.g. for objects being constructed.
        /// </summary>
-        private PooledDictionary<object, PlaceholderLocal> _placeholderLocalsOpt;
+        private ImmutableDictionary<object, PlaceholderLocal> _placeholderLocalsOpt;


private ImmutableDictionary<object, PlaceholderLocal> _placeholderLocalsOpt; [](start = 8, length = 76)

I do not believe there is any advantage in changing the type of this field. #Closed

AlekseyTs · 2019-05-29T21:28:34Z

src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs

-            ArrayBuilder<(BoundReturnStatement, TypeWithAnnotations)> returnTypesOpt,
            VariableState initialState,
-            Dictionary<BoundExpression, (NullabilityInfo, TypeSymbol)> analyzedNullabilityMapOpt)
+            ImmutableArray<(BoundReturnStatement, TypeWithAnnotations)> returnTypesOpt,


ImmutableArray<(BoundReturnStatement, TypeWithAnnotations)> returnTypesOpt [](start = 12, length = 74)

What is the point in passing this array in? Would we ever use its content during analysis? #Closed

AlekseyTs · 2019-05-29T21:33:17Z

src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs

-                foreach (var pair in initialState.VariableTypes)
+
+                var variableTypesBuilder = PooledDictionary<Symbol, TypeWithAnnotations>.GetInstance();
+                foreach (var (key, value) in initialState.VariableTypes)


initialState.VariableTypes [](start = 45, length = 26)

Isn't this already an immutable dictionary? Can't we just use it as is? #Closed

AlekseyTs · 2019-05-29T21:36:46Z

src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.Snapshotter.cs

+                }
+            }
+
+            internal ImmutableDictionary<BoundNode, Snapshot> Snapshots => _snapshotMap.ToImmutableDictionary();


_snapshotMap [](start = 75, length = 12)

Should we handle null here? #Closed

No longer necessary: if there's a SnapshotManager, there is no null chance.

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

AlekseyTs · 2019-05-31T21:37:29Z

src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.SnapshotManager.cs

+                    _currentSymbol = symbol.ContainingSymbol;
+#endif
+                    _walkerStates.SetItem(_currentWalkerSlot, stableState);
+                    if (_initialSymbol.Equals(symbol))


_initialSymbol.Equals(symbol) [](start = 24, length = 29)

Should be able to use reference equality here #Closed

AlekseyTs · 2019-05-31T21:42:33Z

src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.SnapshotManager.cs

+                    _currentSymbol = symbol.ContainingSymbol;
+#endif
+                    _walkerStates.SetItem(_currentWalkerSlot, stableState);
+                    if (_initialSymbol.Equals(symbol))


_initialSymbol.Equals(symbol) [](start = 24, length = 29)

I think we can get rid of _initialSymbol field. Here we can check if _currentWalkerSlot == 0 instead and it doesn't look like it adds much value elsewhere. #Closed

AlekseyTs · 2019-05-31T21:44:19Z

src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.SnapshotManager.cs

+                    if (_initialSymbol.Equals(symbol))
+                    {
+                        Debug.Assert(_currentWalkerSlot == 0);
+                        _currentWalkerSlot = -1;


_currentWalkerSlot = -1; [](start = 24, length = 24)

It feels like we would want to reset _currentSymbol to null as well. #Closed

And _initialSymbol too.

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

AlekseyTs · 2019-05-31T21:50:08Z

Done with review pass (iteration 6) #Closed

cston · 2019-05-31T22:33:28Z

src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.SnapshotManager.cs

+                    _currentSymbol = symbol;
+#endif
+                    _initialSymbol ??= symbol;
+                    var previousSlot = _currentWalkerSlot;


previousSlot [](start = 24, length = 12)

Not used. #Resolved

…evert some changes in this PR to original state.

333fred · 2019-05-31T22:49:34Z

@AlekseyTs @cston addressed feedback.

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

333fred · 2019-06-02T03:41:22Z

src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.SnapshotManager.cs

+
+                internal int EnterNewWalker(Symbol symbol)
+                {
+                    Debug.Assert(symbol is object);


@AlekseyTs unfortunately the assertion that this incoming Symbol is related to the current symbol proved to not be true in bad code scenarios like this one: https://github.com/dotnet/roslyn/blob/master/src/Compilers/CSharp/Test/Semantic/Semantics/LookupTests.cs#L1540-L1555.

AlekseyTs · 2019-06-02T17:59:28Z

src/Compilers/CSharp/Portable/BoundTree/UnboundLambda.cs

                // will be called again from NullableWalker.ApplyConversion when the
                // BoundLambda is converted to an anonymous function.
                // https://github.com/dotnet/roslyn/issues/31752: Can we avoid generating extra
-                // diagnostics? And is this exponential when there are nested lambdas?


? [](start = 85, length = 1)

Dropped a "?" #Closed

cston · 2019-06-03T15:47:05Z

src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs


+        private void SnapshotWalkerThroughConversionGroup(BoundConversion conversionOpt, BoundExpression convertedNode)
+        {
+            var conversionGroup = conversionOpt?.ConversionGroupOpt;


var [](start = 12, length = 3)

Consider returning early if _snapshotBuilderOpt is null. #Closed

cston · 2019-06-03T15:49:54Z

src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs

            return operandType;
        }

+        private void SnapshotWalkerThroughConversionGroup(BoundConversion conversionOpt, BoundExpression convertedNode)


BoundConversion conversionOpt [](start = 58, length = 29)

All callers pass expr as BoundConversion. Consider changing this argument to BoundExpression and checking for BoundConversion here. (We're already performing that check inside the while.) #Closed

cston · 2019-06-03T15:53:05Z

src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.DebugVerifier.cs


            public override BoundNode Visit(BoundNode node)
            {
+                // Ensure that we always have a checkpoint for every BoundExpression in the map


checkpoint [](start = 48, length = 10)

"snapshot" #Closed

cston · 2019-06-03T15:56:04Z

src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.SnapshotManager.cs

+        }
+
+        /// <summary>
+        /// Contains the shared state state used to restore the walker at a specific point


state state [](start = 32, length = 11)

Typo #Closed

cston · 2019-06-03T15:59:48Z

src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.SnapshotManager.cs

+        private readonly struct Snapshot
+        {
+            internal readonly LocalState VariableState;
+            internal readonly int GlobalStateIndex;


Global [](start = 34, length = 6)

Consider using "Shared" rather than introducing a different term. #Closed

cston · 2019-06-03T16:00:21Z

src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker_Patterns.cs

            foreach (var arm in node.SwitchArms)
            {
                SetState(!arm.Pattern.HasErrors && labelStateMap.TryGetValue(arm.Label, out var labelState) ? labelState.state : UnreachableState());
+                // https://github.com/dotnet/roslyn/issues/35836 Is this where we want to take the checkpoint?


checkpoint [](start = 99, length = 10)

"snapshot" #Closed

cston · 2019-06-03T16:17:52Z

src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.SnapshotManager.cs

+            /// <summary>
+            /// The int key corresponds to <see cref="Snapshot.GlobalStateIndex"/>.
+            /// </summary>
+            private readonly ImmutableArray<SharedWalkerState> _walkerGlobalStates;


ImmutableArray _walkerGlobalStates [](start = 29, length = 53)

Is this array necessary? If SharedWalkerState were a class, could we use a reference to SharedWalkerState in Snapshot rather than an index into this array? #WontFix

We could potentially do that, but I'm going to leave this as is. I think making SharedWalkerState a class would probably introduce more garbage than it would save.

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

If the concern is allocations, the fields of SharedWalkerState could be copied into Snapshot.

In reply to: 289940273 [](ancestors = 289940273,289925758)

cston · 2019-06-03T16:20:07Z

src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.SnapshotManager.cs

+            /// closest position before a given position.
+            /// </summary>
+            private sealed class DescendingIntComparer : IComparer<int>
+            {


{ [](start = 12, length = 1)

Consider adding a singleton instance. #Closed

333fred · 2019-06-03T18:11:24Z

@AlekseyTs @cston addressed feedback.

cston · 2019-06-03T18:16:23Z

src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.SnapshotManager.cs

+            /// <summary>
+            /// The int key corresponds to <see cref="Snapshot.SharedStateIndex"/>.
+            /// </summary>
+            private readonly ImmutableArray<SharedWalkerState> _walkerGlobalStates;


Global [](start = 70, length = 6)

"Shared"?

cston · 2019-06-03T18:16:51Z

src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.SnapshotManager.cs

+                    return null;
+                }
+
+                var globalState = _walkerGlobalStates[incrementalSnapshot.SharedStateIndex];


globalState [](start = 20, length = 11)

"sharedState"?

cston · 2019-06-03T18:17:37Z

src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.SnapshotManager.cs

+            internal readonly LocalState VariableState;
+            internal readonly int SharedStateIndex;
+
+            internal Snapshot(LocalState variableState, int globalStateIndex)


global [](start = 60, length = 6)

"shared"

AlekseyTs

LGTM (iteration 9)

333fred · 2019-06-03T22:43:38Z

@cston I'll address those comments in the next followup PR.

Add initial checkpointing to the nullable walker.

38e9d21

333fred added Area-Compilers Feature - Nullable Semantic Model Nullable Semantic Model Issues labels May 21, 2019

333fred requested review from AlekseyTs and gafter May 21, 2019 20:37

333fred requested a review from a team as a code owner May 21, 2019 20:37

333fred commented May 21, 2019

View reviewed changes

cston reviewed May 21, 2019

View reviewed changes

Refactor NullableWalker to make it use as much immutable state as pos…

c64cf66

…sible, and add a caching strategy to the snapshotter to reduce garbage creation for every node. Addressed minor PR feedback items as well.

AlekseyTs reviewed May 29, 2019

View reviewed changes

AlekseyTs reviewed May 31, 2019

View reviewed changes

cston reviewed May 31, 2019

View reviewed changes

Sort the dictionary in descending order by using a custom comparer. R…

893239e

…evert some changes in this PR to original state.

333fred force-pushed the checkpoint-walker branch from 5a246c3 to 893239e Compare May 31, 2019 22:48

Fix test issues.

14f7bfd

333fred commented Jun 2, 2019

View reviewed changes

AlekseyTs reviewed Jun 2, 2019

View reviewed changes

cston reviewed Jun 3, 2019

View reviewed changes

Addressed PR feedback.

f011803

cston reviewed Jun 3, 2019

View reviewed changes

cston approved these changes Jun 3, 2019

View reviewed changes

AlekseyTs approved these changes Jun 3, 2019

View reviewed changes

333fred merged commit 2f8cc93 into dotnet:master Jun 3, 2019

333fred deleted the checkpoint-walker branch June 3, 2019 22:43

yyjdelete mentioned this pull request Aug 9, 2019

NullReferenceException in NullableWalker.TakeIncrementalSnapshot when input some invalid code #37868

Closed

Conversation

333fred commented May 21, 2019

Uh oh!

333fred May 21, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

333fred May 21, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cston May 21, 2019 • edited by 333fred Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cston May 21, 2019 • edited by 333fred Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cston May 21, 2019 • edited by 333fred Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cston May 21, 2019 • edited by 333fred Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cston May 21, 2019 • edited by 333fred Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AlekseyTs May 29, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AlekseyTs May 29, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AlekseyTs May 29, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AlekseyTs May 29, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AlekseyTs May 29, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AlekseyTs May 29, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AlekseyTs May 29, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AlekseyTs May 29, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

333fred May 21, 2019 •

edited

Loading

333fred May 21, 2019 •

edited

Loading

cston May 21, 2019 •

edited by 333fred

Loading

cston May 21, 2019 •

edited by 333fred

Loading

cston May 21, 2019 •

edited by 333fred

Loading

cston May 21, 2019 •

edited by 333fred

Loading

cston May 21, 2019 •

edited by 333fred

Loading

AlekseyTs May 29, 2019 •

edited

Loading

AlekseyTs May 29, 2019 •

edited

Loading

AlekseyTs May 29, 2019 •

edited

Loading

AlekseyTs May 29, 2019 •

edited

Loading

AlekseyTs May 29, 2019 •

edited

Loading

AlekseyTs May 29, 2019 •

edited

Loading

AlekseyTs May 29, 2019 •

edited

Loading

AlekseyTs May 29, 2019 •

edited

Loading

AlekseyTs May 29, 2019 •

edited

Loading

AlekseyTs May 29, 2019 •

edited

Loading

AlekseyTs May 29, 2019 •

edited

Loading

AlekseyTs May 29, 2019 •

edited

Loading

AlekseyTs May 29, 2019 •

edited

Loading

AlekseyTs May 29, 2019 •

edited

Loading

AlekseyTs May 29, 2019 •

edited

Loading

AlekseyTs May 31, 2019 •

edited

Loading

AlekseyTs May 31, 2019 •

edited

Loading

AlekseyTs May 31, 2019 •

edited

Loading

AlekseyTs commented May 31, 2019 •

edited

Loading

cston May 31, 2019 •

edited by 333fred

Loading