Fix data races in sorting and Reverse

[bep]

And add caching to the result.

It is, not surprisingly, also faster:

benchmark                           old ns/op     new ns/op     delta
BenchmarkSortByWeightAndReverse     14104         465           -96.70%

benchmark                           old allocs     new allocs     delta
BenchmarkSortByWeightAndReverse     1              0              -100.00%

benchmark                           old bytes     new bytes     delta
BenchmarkSortByWeightAndReverse     32            0             -100.00%

Fixes #1293

[bep]

Of course, just making copies on every sort and reverse will also fix the main issue here, but at a slight cost:

benchmark                           old ns/op     new ns/op     delta
BenchmarkSortByWeightAndReverse     14304         19858         +38.83%

benchmark                           old allocs     new allocs     delta
BenchmarkSortByWeightAndReverse     1              3              +200.00%

benchmark                           old bytes     new bytes     delta
BenchmarkSortByWeightAndReverse     32            5152          +16000.00%

@tatsushid @spf13 ... any thoughts?

[spf13]

wow. Thanks for tracking this down. I think caching is a big win, especially for large sites. I'm not sure I fully understand the root issue. Is it that the sort / reverse isn't safe for concurrency and consequently the order isn't right/ items get lost?

It seems the best approach would be to make it concurrently safe (mutexes or channels would work well). Then cache the results.

Is this what you were looking for?

[bep]

There are several issues in play here:

• The symptom in this case is "lost pages" caused by a data race where Reverse modified the slice while ByDateread it.
• There some methods that operate on Pages that does restructuring of the slice.
• Most use cases use this modified slice only once (in the index template)
• In this case, the single template had a list of recent posts in the side-bar, so the Data.Pages.ByDate.Reverse were executed 300 times in parallell => data race.
• Then there are, of course, variants of the above, people trying do multiple sorting of the same data etc.

My fix is this:

• A copy is made of the slice before doing any structural changes and all writes are locked. This fixes the concurrency issues / data races etc.
• In addition to the above, the result of the restructuring is cached (with a read/write mutex to protect the cache). In normal cases (small blogs), there are very minute savings from caching this result, but it just sounds ineffective to sort and reverse the slice 300 times with the same result.

I have thought more on this, and will polish up my fix, as it makes sense to me.

[bep]

Merged in a9c5133

[github-actions]

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.