[WIP] iseq. A fast seq implementation. #2587

manofstick · 2017-03-11T19:10:43Z

This is meant as a replacement for #1570.

Given the currently unpalatable nature of the #1570 PR, a more accommodating version is being prepared in this PR. The plan is to not replace the existing version of seq, but to provide the new version in a side-by-side version in order to minimize the risk associated with such a change.

This does obviously increases the surface area of FSharp.Core, but during the development of #1570 it became apparent that exposing the internals of the implementation, where they were exposed as inline functions was a useful and fruitful exercise. In a blog post, @mrange analysed performance of this inlinable version and found it as quite good. The name change from #1570's composer to the simpler iseq is reflecting this usage.

If this was never intended to replace the internals of seq I would think it be sufficient to release this as a separate library, but the goal of this is to replace seqs implementation. Personally I would like to see the current seq internals replaces by by iseq within a version or two of this iseq version hitting release status. I don't think all downstream code should be littered with inlined code unnecessarily.

Please, if this is not going to fly, let me know sooner rather than later. My time is to do this basically non-existent at the moment due to other commitments. My coding hours are between 4:30am and 7:00am on the weekends... This is not sustainable (well for me anyway!)

cc: @dsyme @KevinRansom @rmunn @smoothdeveloper @liboz @jackmott @asik @7sharp9 @PatrickMcDonald @mexx @cloudRoutine @mrange @isaacabraham

manofstick · 2017-03-12T08:13:54Z

@dsyme (or anyone else...)

Don, as I know you want to minimize diffs in order to make them easier to review, would you think it would be of use to commit this PR as it currently stands as it currently does nothing, other than splitting seq.fs into two parts, but then means that further work should be easier to isolate?

smoothdeveloper · 2017-03-12T08:16:48Z

Sounds sensible (to me) if this current PR is splitting the file with no changes, it sounds better to have this as a base.

cloudRoutine · 2017-03-12T08:21:02Z

@manofstick i think you should commit the seq module with the internals replaced to make sure it passes the test suite.

If the decision ends up being that it won't replace the internals of seq, just pop the commit.

manofstick · 2017-03-12T08:35:41Z

@cloudRoutine

Well it's not just one commit, and it's not just one place where this will occur. It ends up modifying surface area, the bsl test files, seq, iseq, new tests for iseq, maybe more, The point is, it's messy. If it can be a staged release. First part being this, as it stands, split seq file. The second being the creation of iseq with associated tests. the third being the replacement of seq then it limits the cognitive load required to grok it.

cloudRoutine · 2017-03-12T08:41:10Z

I didn't mean all of those other files, I was only thinking of the seq module itself so those tests could run. I was under the impression that all the normal tests would run without issue if you're only changing the internals of the implementations.

I should probably get started on that test organization & cleanup PR then

manofstick · 2017-03-13T09:11:36Z

OK didn't get as fast as I wanted this weekend (wanted to make a start replicating Seq tests for ISeq), but at least we're there slightly...

So as a rudimentary test it will now run:

open System.Diagnostics

let fast () =
    for i = 1 to 5 do
        let sw = Stopwatch.StartNew ()

        let x =
            ISeq.init 1000000 int64 
            |> ISeq.map (fun x -> x * x)
            |> ISeq.sum

        printfn "Fast: %dms\t(checksum=%d)" sw.ElapsedMilliseconds x

let slow () =
    for i = 1 to 5 do
        let sw = Stopwatch.StartNew ()

        let x =
            Seq.init 1000000 int64 
            |> Seq.map (fun x -> x * x)
            |> Seq.sum

        printfn "Slow: %dms\t(checksum=%d)" sw.ElapsedMilliseconds x

[<EntryPoint>]
let main argv =
    fast ()
    printfn ""
    slow ()

    0

with the output:

Fast: 31ms      (checksum=333332833333500000)
Fast: 7ms       (checksum=333332833333500000)
Fast: 6ms       (checksum=333332833333500000)
Fast: 6ms       (checksum=333332833333500000)
Fast: 6ms       (checksum=333332833333500000)

Slow: 171ms     (checksum=333332833333500000)
Slow: 164ms     (checksum=333332833333500000)
Slow: 166ms     (checksum=333332833333500000)
Slow: 161ms     (checksum=333332833333500000)
Slow: 165ms     (checksum=333332833333500000)

(init was a particular terrible Seq implementation, so this performance improvement is not really expected across the board, but hey, good for a demo!)

dsyme · 2017-03-14T15:00:50Z

Don, as I know you want to minimize diffs in order to make them easier to review, would you think it would be of use to commit this PR as it currently stands as it currently does nothing, other than splitting seq.fs into two parts, but then means that further work should be easier to isolate?

Yes, thanks, good idea

manofstick · 2017-03-15T09:33:45Z

manofstick · 2017-03-23T19:27:56Z

@dsyme @KevinRansom

Ok, well I have a few things to do, but there is nothing stopping a general review I don't think.

So I copied the unit tests from seq and modified them the minimum that I could to make them compatible with iseq (I'm not wedded to that name). The main change, other than search and replace, was removing the tests for null, due to ISeq being a f# defined interface (hey, if you think it could escape these walls then maybe I should think bigger!)

Some things are quite idiomatic here (to say the least...) And some of them could be simplified with modifications to the compiler. I use the Values struct (which now could be changes to System.ValueTuple except it doesn't compile - I didn't check, but maybe the proto compiler needs to be updated?) of various arity to insert member variables into the object expressions. This is to minimise objects created, as captured variables all become Ref fields rather than members.

I think that is probably the biggest of the idioms used. But the general theme had been to minimise objects created so as to optimise the common case of small seqs not being punished by this change.

Oh, and the whole thing is a bit more complex than I would like (things like PipeIdx, handling of skipping) as they were introduced so that I could retain full compatibility with the existing Seq codebase. These nuances are probably not fully on display now that this current iteration isn't actually relaxing seq, but would obviously be important in the future (and hopefully captured by the coping of the Seq unit tests).

Anyway I still have a few tweaks to go (as the surface area was filled out just with copies from seq and so can be improved upon) but I don't think that stops some form of review at the higher level.

dsyme · 2017-03-24T09:09:55Z

src/fsharp/FSharp.Core/iseq.fsi

+            val mutable _2: 'b
+            val mutable _3: 'c
+
+        /// <summary>PipeIdx denotes the index of the element within the pipeline. 0 denotes the


You don't need to put <summary> tags if there is nothing but a summary

dsyme · 2017-03-24T09:10:11Z

src/fsharp/FSharp.Core/iseq.fsi

+        /// source of the chain.</summary>
+        type PipeIdx = int
+
+        type IOutOfBand =


This needs a /// comment

dsyme · 2017-03-24T09:12:08Z

src/fsharp/FSharp.Core/iseq.fsi

+            abstract OnComplete : PipeIdx -> unit
+
+        [<AbstractClass>]
+        type TransformFactory<'T,'U> =


Add /// comments to all types and methods, thanks

dsyme · 2017-03-24T09:25:44Z

src/fsharp/FSharp.Core/iseq.fs

+
+                    override this.OnComplete _ =
+                        if this.State then
+


I'm reticent about such huge amounts of inline in FSharp.Core. Is the idea that ISeq will be public? Huge amounts of inlining can give performance gains at the cost of large code generation.

In general we should only inline when there is a known, identified performance gain that stems from a particular type-specialization or compiler optimization (e.g. combining function compositions).

Since the body of most of these functions is an object expression I'm not sure if any specific optimization or type specialization will kick in?

Note that we don't tend to mark functions inline on the hope that a small explicit lambda is supplied as function argument and that this will in turn be inlined in the body. I'd like to implement some kind of "conditional inlining" eventually that covers this case.

Either way, for your performance comparisons you should probably use like-for-like inlining between existing Seq functions and ISeq functions (so ISeq isn't getting an "advantage" through extra inlining)

Well the "I" in ISeq was stands for inline :-)

So originally these were inline as ISeq was an internal type that I was just using as a helper for my replacement of Seq, so they were inlined deliberately so that they would then inhabit the Seq module, but after testing performance version Nessos's Streams, I found that the only reason ISeq was slower was because it wasn't inlining functions (as it was being accessed through Seq) so I made the ISeq interface public and that's where we are.

So if we do the next phase of this project, which is to gut Seq and replace it as stub calls to ISeq
(as discuessed here) then I think the overkill of inline in ISeq is fine. Most users not care about ISeq; they will get the majority of the performance boost without touching a line of code.

For those that do want to squeeze the last bit out of their piplelines the can start passing ISeqs around and they get inlined code...

dsyme · 2017-03-24T13:15:51Z

src/fsharp/FSharp.Core/iseq.fsi

+        /// <summary>Values is a mutable struct. It can be embedded within the folder type
+        /// if three values are required for the calculation.</summary>
+        [<Struct; NoComparison; NoEquality>]
+        type Values<'a,'b,'c> =


I think this is only ever instantiated once. But maybe in the future it will be more

Values can be replaced by ValueTuple (as it is serves exactly the same purpose of evil mutable value type :-) but the proto-compiler doesn't know about it yet? Maybe? Or maybe I'm just stuffing something up.

All of these could be removed if the F# compiler made proper members of captured variables within object expressions. So that might be a possible thing to do before locking this public facing API out there...

dsyme · 2017-03-24T13:21:17Z

src/fsharp/FSharp.Core/iseq.fs

+                    member __.Iterate (outOfBand:Folder<'U,'Result,'State>) (consumer:Activity<'T,'U>) =
+                        let array = array
+                        let rec iterate idx =
+                            if idx < array.Length then  


Should this be an actual while loop? Perhaps it will help prevent array bounds checking

Who knows the magic of the runtime? But if you try and run this then under 32-bit the for loop is the slowest, with the while and recursive function about the same, but under 64-bit then the for loop and while are about the same and the recursive function is fastest!

Anyway, on the highlighted code I either found no difference, or the recursive function was better - can't remember, but I do remember trying.

dsyme · 2017-03-24T13:22:38Z

src/fsharp/FSharp.Core/iseq.fs

+                        let rec iterate idx =
+                            if idx < array.Length then  
+                                consumer.ProcessNext array.[idx] |> ignore
+                                if outOfBand.HaltedIdx = 0 then


I suppose there are two virtual calls in the loop here. Are they both necessary? Could ProcessNext return the HaltedIdx? or does that make no sense..

Hmmmm... I'm trying to remember why the out of band object was used rather than just paramertising ProcessNext... it was fairly early in the evolution of this so I can't honestly remember. I'm pretty sure I considered it - in fact I think that was how I originally coded it; maybe I'll go source code spelunking to see if I can find out...

dsyme · 2017-03-24T13:22:55Z

src/fsharp/FSharp.Core/iseq.fs

+                        iterate 0
+
+            [<Struct;NoComparison;NoEquality>]
+            type resizeArray<'T> (array:ResizeArray<'T>) =


capitalize resizeArray?

dsyme · 2017-03-24T13:23:51Z

src/fsharp/FSharp.Core/iseq.fs

+                        if outOfBand.HaltedIdx = 0 && finalIdx = System.Int32.MaxValue then
+                            raise <| System.InvalidOperationException (SR.GetString(SR.enumerationPastIntMaxValue))
+
+            let execute (createFolder:PipeIdx->Folder<'U,'Result,'State>) (transformFactory:TransformFactory<'T,'U>) pipeIdx (executeOn:#IIterate<'T>) =


Please add /// comments on many of these - but particularly execute and executeThin to explain the difference

dsyme · 2017-03-24T13:25:12Z

src/fsharp/FSharp.Core/iseq.fs

+                        interface System.IDisposable with
+                            member x.Dispose () = () }
+
+            type EnumerableDecider<'T>(count:Nullable<int>, f:int->'T, pipeIdx:PipeIdx) =


/// please. When I read this first time I had no idea what a Decider is. It seems to be only used for init?

Yeah I originally had all the Enumerable wrappers in their own modules (of which EnumerableDecider still is) and so this was in Init.EnumerableDecider - deciding which type of enumerable to use for the init call. But then I had to start to and together some enumerables in the main module, so this has kind of lost its meaning.

Anyway, I think I'll remove all the modules (just keep the main Enumerables module) and then name the types without it.

dsyme · 2017-03-24T13:28:34Z

@manofstick So is the plan to actually make this replace Seq? Or were you thinking of actually having the new ISeq type added? thanks

dsyme · 2017-03-24T14:43:50Z

Hi @manofstick

This is fantastic work, and the technique you are using is now much clearer.

I've been discussing the criteria under which we would accept this PR with Nick Palladinos and others. Here's our thinking

No new types are exposed in the users's normal programming model. Some "internal" types may need to be exposed for the purposes of inlining, but they would become a permanent part of the API of the library, never to change
Any new surface area of the exposed types-machinery that may affect the future evolution of the library and has to be very carefully assessed
After this PR, Seq needs to be significantly faster not only than the current FSharp.Core Seq but also from the C# IEnumerable/IEnumerator LINQ implementation. Some very small degradations would be ok, but "almost never slower" is crucial
If, after this PR, Seq were also performance-comparable to Nessos Streams that would also be great
The new Seq implementation must be fully compatible
[*] No runtime code generation

The performance test matrix for this must cover a matrix of

a range of input types
every function in the Seq.* programming model
empty, small and big inputs
special inputs known to Seq e.g. lists, arrays

It's a big performance test matrix but I think you've done quite a lot of the necessary perf testing already.

What do you think? Do those criteria sound right?

manofstick · 2017-03-25T02:18:20Z

@manofstick So is the plan to actually make this replace Seq? Or were you thinking of actually having the new ISeq type added? thanks

Well this is really the discussion I want to start, and so it is an open question.

First of all ISeq as it stands is not a replacement for Seq. The Seq interface is out there and it something that creates and consumes IEnumerable<>s, and I think that is right and correct. In the original incarnation of what became ISeq it was embedded within Seq and I was casting on the way in and on the way out, and so to make it cleaner for myself I moved all the implementation to a separate file, and the Seq really just became stub functions that looked like:

    [<CompiledName("Filter")>]
    let filter<'T> (f:'T->bool) (source:seq<'T>) : seq<'T> =
        source |> ISeq.ofSeq |> ISeq.filter f |> Upcast.enumerable

Or slightly more complex case where it also handled Adapt

    [<CompiledName "IterateIndexed">]
    let iteri f (source:seq<'T>) =
        let f = OptimizedClosures.FSharpFunc<_,_,_>.Adapt f
        source |> ISeq.ofSeq |> ISeq.iteri (fun idx a -> f.Invoke(idx,a))

But then I imaged that it might be useful to use the ISeq directly to avoid the casting in and out, as well as also offering the advantage of inlining to those people who wanted to push performance.

It's not the easiest to see from the lines, but in @mrange's analysis from his blog post where SeqComposer is the use through Seq and SeqComposer2 is the use through ISeq that using the ISeq implemenation you get (at least in this case!) better than Nessos Streams performance (and that it performs significantly better for small collections over everything except Linq)

So, we could not expose it, only really having to expose the Activity pipeline, and just have all the ISeq stuff as the internal implementation for Seq, but there is some value in exposing it. Anyway, those calls are above my pay-grade!

manofstick · 2017-03-25T02:27:51Z

What do you think? Do those criteria sound right?

Indeed it does!

...and would definitely love for some extra heads to be thinking about the API side of things. I have had great help on the implementation (and motivational!) side from @liboz, @cloudRoutine and @smoothdeveloper (and others), but some thoughts into crafting the final visible space would be greatly appreciated.

cloudRoutine · 2017-03-28T02:36:03Z

@manofstick I can't wait to turn this into an ISeq https://github.com/Microsoft/visualfsharp/blob/master/vsintegration/src/FSharp.Editor/Structure/Structure.fs

😀

manofstick · 2017-03-28T04:02:30Z

@cloudRoutine

Unless the seq computational expression are modified (which could be done at some future stage), then it probably won't help... But that's just from a quick eyeballing...

cloudRoutine · 2017-03-28T04:04:58Z

I was expecting we'd build an iseq{ ... } instead

manofstick · 2017-03-28T04:15:15Z

Possible, although seq computational expressions compile into a fairly efficient state machine (there is still work there that can make them faster, but pretty good). Now you could build iseq {} just on top of ISeq without compiler support, but I doubt that would be faster; so probably means you'd have to do a fair amount of work to get compiler support for iseq...

vasily-kirichenko · 2017-03-28T05:12:52Z

you could build iseq {} just on top of ISeq without compiler support, but I doubt that would be faster

wow. So all this work is done to make Seq.xxx combinators work less than 2x faster?.. If you do need fast lazy sequences, use raw linq at places or switch to nessos.

manofstick · 2017-03-28T05:44:36Z

wow. So all this work is done to make Seq.xxx combinators work less than 2x faster?.. If you do need fast lazy sequences, use raw linq at places or switch to nessos.

Someone got up on the wrong side of bed this morning!!

Anyway, I didn't say anything about actual performance - I was comparing seq computational expressions which are compiler generated to a possible computation expression generated with the ISeq library as it stands and was just putting a finger in the air around performance.

And as for what the point of this code is, it gives comparable performance to Nessos streams without any clients having to change a line of code. The ISeq form is just to try and get this across the line as @dsyme was, rightly, concerned about the full delta of changing everything at once. So yeah, if you think the performance of Nessos streams is insufficient, then you will think the performance of this work is insufficent.

And lastly, believe it or not, I actually do this for fun...

mrange · 2017-03-28T18:39:14Z

In the performance measurements I have done the work done by Paul W performs somewhat better than Nessos especially if one uses the Composer directly. In addition, there are myriad libraries out that there that out-performs the standard F# Seq (like LINQ). I have written a few myself out of desperation. However, I think it is very valuable to have a lazy sequence in the FSharp.Core that offers good performance and doesn’t allocate objects needlessly. IMHO this is one of the most important improvements to Fsharp.Core that I know of. Mårten

abelbraaksma · 2017-08-30T01:49:07Z

@manofstick, @dsyme, I really love this addition and what is brings, I have many necessary micro-optimizations in my code for Seq and I would be very interested to see how this works out.

Any idea when this is going to be moved forward and/or becomes available in a (nightly) release to be tested in the wild?

manofstick · 2017-08-30T07:51:27Z

Hi @abelbraaksma - nice to have some interest!

The current plan is ISeq itself won't be exposed, but rather through #2745 (it is branched off the head of this PR) where it will be replacing the guts of Seq.

I'm currently working on fixes from code review's in that PR, but I really need to find half a day without distraction to do the next iteration of that - hopefully I'll get onto that soon; i.e. next week or so? But then there will probably be another round of review from @dsyme - I assume we're getting towards a release, but it is quite a change that requires some careful, cautious deliberation...

So sorry I can't pin down a date, but I'll try harder to make some time to at least get the ball out of my court.

abelbraaksma · 2017-08-30T10:42:50Z

@manofstick, that's all good news! If there's something specific you'd like to see tested, I can help. Does that mean this thread should be closed in favor of #2745?

dsyme · 2017-08-30T11:26:08Z

If there's something specific you'd like to see tested, I can help.

@abelbraaksma I'd like this reviewed line by line. And I mean every line, understood and reviewed, by multiple contributors. If you'd like to be one of those, then start at the top and work through :)

Another great thing would be to get some code coverage information for this. We don't regularly use code coverage tools in this repo, but I'd particularly like it for FSharp.Core

Does that mean this thread should be closed in favor of #2745?

Yes - @manofstick I'd prefer one PR at this point

abelbraaksma · 2017-08-30T16:39:33Z

Another great thing would be to get some code coverage information for this. We don't regularly use code coverage tools in this repo, but I'd particularly like it for FSharp.Core

@dsyme: on a more general note, I can see if I could leverage my build system (a TC server), which has default code-coverage for several test suites. Or I could run it locally with NCrunch, apart from some of the inlined methods/functions it usually finds just about every line quite well and turns them green (plus lists which tests cover which line) in the editor. I use it for my own F# projects to get a near 100% code coverage with tests.

I haven't yet tried to build FSharp.Core locally yet, but I assume it shouldn't be too hard.

manofstick · 2017-09-02T07:40:36Z

Closing in deference to #2745

dsyme · 2017-09-04T16:31:47Z

@abelbraaksma

@dsyme: Or I could run it locally with NCrunch, apart from some of the inlined methods/functions it usually finds just about every line quite well and turns them green (plus lists which tests cover which line) in the editor. I use it for my own F# projects to get a near 100% code coverage with tests.

If we could put automated coverage reporting into the CI that would be awesome

msftclas added the cla-already-signed label Mar 11, 2017

manofstick mentioned this pull request Mar 12, 2017

Splitting seq.fs #2598

Merged

manofstick force-pushed the manofstick-iseq branch from 237585d to 4ac2bb1 Compare March 15, 2017 05:53

This was referenced Mar 16, 2017

Use new String.Join overload added in .Net 4.0 (~3x faster) #2570

Merged

Equality operator causes boxing on value types #526

Closed

manofstick force-pushed the manofstick-iseq branch 2 times, most recently from 7f2f29e to 2169bd9 Compare March 23, 2017 07:56

dsyme reviewed Mar 24, 2017

View reviewed changes

dsyme mentioned this pull request Mar 24, 2017

[WIP] Seq Composer #1570

Closed

99 tasks

manofstick force-pushed the manofstick-iseq branch from a2a2136 to c2d6fd0 Compare March 28, 2017 04:04

manofstick added 21 commits August 16, 2017 16:46

Added check on negative input for take

9941af4

Natively support ISeq on list

aeeb049

Added list specific overloads

ba95b23

Optimization for 0/1 element arrays

8c16ea8

Map/Set use Seq.fold to utilize ISeq implementation

bc92c68

ISeq.length optimized for list.length

919a2a1

Removed Upcast hack due to dotnet#2972

b8335bc

Consolidated common factory code

04f5eff

Converted TransformFactory into interface

5b8aa9b

Moved list/array before seq in compilation order

1179590

Removed duplicate code in Thin implementations

278742c

Added EnumerableBase.GetRaw

a38fb22

made EnumerableBase internal

3331b82

Fix surface area

e51ce8b

Moved ISeq.length into Basics

ffadb9f

Added ISeq support in Array.ofSeq

c4f8b43

Added IReadOnlyCollection to Array.ofSeq

eb5299f

Merged EnumeratorBase and Result

3fd1243

Use list for small ISeqs

09e3c98

Thin direct calls to IEnumerable.GetEnumerator

f010085

Provided an optimized GetEnumerator for scan

ded9bb7

manofstick force-pushed the manofstick-iseq branch from 86b7a6a to ded9bb7 Compare August 16, 2017 07:00

manofstick closed this Sep 2, 2017

[WIP] iseq. A fast seq implementation. #2587

[WIP] iseq. A fast seq implementation. #2587

Conversation

manofstick commented Mar 11, 2017

manofstick commented Mar 12, 2017

smoothdeveloper commented Mar 12, 2017

cloudRoutine commented Mar 12, 2017 • edited Loading

manofstick commented Mar 12, 2017 • edited Loading

cloudRoutine commented Mar 12, 2017

manofstick commented Mar 13, 2017

dsyme commented Mar 14, 2017

manofstick commented Mar 15, 2017 • edited Loading

manofstick commented Mar 23, 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

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

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

dsyme commented Mar 24, 2017

dsyme commented Mar 24, 2017 • edited Loading

manofstick commented Mar 25, 2017

manofstick commented Mar 25, 2017

cloudRoutine commented Mar 28, 2017

manofstick commented Mar 28, 2017

cloudRoutine commented Mar 28, 2017

manofstick commented Mar 28, 2017

vasily-kirichenko commented Mar 28, 2017

manofstick commented Mar 28, 2017

mrange commented Mar 28, 2017 via email • edited by dsyme Loading

abelbraaksma commented Aug 30, 2017

manofstick commented Aug 30, 2017

abelbraaksma commented Aug 30, 2017

dsyme commented Aug 30, 2017

abelbraaksma commented Aug 30, 2017 • edited Loading

manofstick commented Sep 2, 2017

dsyme commented Sep 4, 2017

cloudRoutine commented Mar 12, 2017 •

edited

Loading

manofstick commented Mar 12, 2017 •

edited

Loading

manofstick commented Mar 15, 2017 •

edited

Loading

dsyme commented Mar 24, 2017 •

edited

Loading

mrange commented Mar 28, 2017 via email •

edited by dsyme

Loading

abelbraaksma commented Aug 30, 2017 •

edited

Loading