- Convert SumF functions for Array, Span, and List into a generic function #20

Smurf-IV · 2019-07-30T13:16:22Z

Convert SumF function into a generic function
Add new managed types (sbyte, byte, short, ushort, int, uint, long, ulong)
Add tests for some of the new types
Add new Benchmark tests

- Add new managed types (sbyte, byte, short, ushort, int, uint, long, ulong) - Add tests for some of the new types - Add new Benchmark tests

Smurf-IV · 2019-07-30T16:22:03Z

This fixes #18
[Bug] using SumS on a ushort[] throws System.InvalidCastException #18

- Make sure publics have Intellisense - Add Nullable types - Perform template on source selectors

- Apply template to Spans<t> - Apply Template to List<T>

- Add List benchmarks

- Add SumF API's to redirect IList's to their base types

jackmott · 2019-07-31T13:29:18Z

Hey Simon, wanted to say thanks for all the experiments you are doing here. I'll need to some time to review/test some of this stuff out, and I'm pretty busy with family/work all the time so it might take a while. I'll try to take a deep look in a few days and let you know what I think.

…e types

Smurf-IV · 2019-07-31T14:43:04Z

wanted to say thanks for all the experiments you are doing here.

No worries, Now that I have the templates in place, I can burn through the other types pretty quickly

- Add inlining hint to AverageIList

jackmott · 2019-07-31T14:50:37Z

I would hold off on doing too much work until I've had some time to review and test. I don't want you to write a million lines of code and then I end up not using them!

- Fix code to make it appear "As fast" Via the use of the "Coming out faster" acum = accum + nextVal method - Add ReadOnlySpan to `SumF` api's

- Done for Aggregate - Add All Span types - Add ReadOnlySpan - Reduce code in SumList

- Fix copy paste error - Add better intellisense text

…ower than just calling foreach -> Why?

…lower than just calling foreach -> Why?

…ReadOnlyList<T> - Change over to IReadOnlyList

…dOnlyList<T> - Implement LastF's the same way as FirstF's

…be implemented - Add Start File

…ReadOnlyList<T> - Use types instead of vars - Breakout Max Benchmarks - Breakout MaxF API's for different types into seperate files

…ReadOnlyList<T> - Split MinF's out into seperate files - Change to NameOf for Selector parameter

… on IReadOnlyList<T> - Replacement to use nameof for source - Split more benchmarks out into seperate relevant files

Smurf-IV · 2019-08-06T14:03:11Z

Hey Simon, wanted to say thanks for all the experiments you are doing here. I'll need to some time to review/test some of this stuff out, and I'm pretty busy with family/work all the time so it might take a while. I'll try to take a deep look in a few days and let you know what I think.

Just found a bog "No-no" in some of the "Optimisations" I've done
When using the List's I have done the following

        int sourceCount = source.Count;
        T2 a = p.Zero();
        checked
        {
            for (int index = 0; index < sourceCount; index++)
            {
                a = p.Add(a, source[index]);
            }
        }

The big "No-No" is on the int sourceCount = source.Count; as the Items in the List could be changed externally "Darn threading !". One way would be to lock the list, another is to put the count Back into the for loop,
I'll go with the latter, I think this may also need to be done for the IReadOnlyList but I'm not sure.

Smurf-IV · 2019-08-06T14:16:01Z

    // Note: Lists can have items added and removed whilst these API's are in use
    // The IReadOnlyList<T> represents a list in which the _number_ and _order_ of list elements is read-only.
    //

… in use - The IReadOnlyList<T> represents a list in which the _number_ and _order_ of list elements is read-only.

jackmott · 2019-08-06T14:31:56Z

int sourceCount = source.Count; is a non optimization on List anyway. Did you mean IList?

Smurf-IV · 2019-08-06T14:36:25Z

int sourceCount = source.Count; is a non optimization on List anyway. Did you mean IList?

No on the List<T> it did show "Some" improvement....

…Benchmark - Add Ratio Comparisons to Benchmarks - Add .Net4.8 Output directories - Explicit call types for Average

… Benchmark - Attempt to sort why Appveyor is unhappy. - Sort out the ratio detection code when all are put back in.

Smurf-IV · 2019-08-07T15:03:05Z

No idea why AppVeyor is complaining, It runs on my machine !!

…er a span Create jackmott#22: [Question] Span_FirstF and List_FirstF are slower than just calling foreach -> Why? Span this jackmott#24: [Enhancement] DefaultIfEmptyF should be implemented Add Span jackmott#25: [Enhancement] Please also target .net4.8 and Benchmark

…in the csproj file...

…ing weird stuff.

ndrwrbgs

From what I see cursorily, it looks like there's a lot more changes than the title suggestions. Perhaps you could send a more focused/isolated review so that each of the changes can be evaluated on it's own merit (rather than one change having to block any benefit from the others due to being coupled)?

If my understanding of part of the change is correct about 'generic function' actually being 'use IReadOnlyList' then I'd be concerned about performance regressions when Array, Span, and List are passed in, as those have compiler/JIT optimizations for iteration.

ndrwrbgs · 2019-08-10T18:17:07Z

LinqFaster/Select.cs

@@ -245,12 +143,12 @@ public static void SelectInPlaceF<T>(this List<T> source, Func<T, int, T> select
        {
            if (source == null)
            {
-                throw Error.ArgumentNull("source");
+                throw Error.ArgumentNull(nameof(source));


isolating the changes would be a major help to reviewers. These, while potentially desirable, provide a lot of noise as they aren't "Convert SumF into a generic function"

ndrwrbgs · 2019-08-10T18:17:42Z

Benchmarks/App.config

        <supportedRuntime version="v4.0" sku=".NETFramework,Version=v4.6.1" />
    </startup>
-  <runtime>


Avoiding these kind of whitespace changes can help focus the review and speed up the review. In addition, it helps reviewers to stay focused so they can more likely catch issues

ndrwrbgs · 2019-08-10T18:18:13Z

LinqFaster/Repeat.cs

@@ -34,7 +34,7 @@ public static T[] RepeatArrayF<T>(T element, int count)
        /// <returns>A sequence that contains a repeated value</returns>
        public static List<T> RepeatListF<T>(T element, int count)
        {
-            var result = new List<T>(count);
+            List<T> result = new List<T>(count);


more 'style' changes that aren't necessary

ndrwrbgs · 2019-08-10T18:19:51Z

LinqFaster/Aggregate.cs

        /// <param name="func">An accumulator function to be invoked on each element</param>
        /// <returns>The final accumulator value</returns>
-        public static TSource AggregateF<TSource>(this TSource[] source, Func<TSource, TSource, TSource> func)


Converting T[] to IReadOnlyList makes it have to call through the interfaces for everything, which involves a vtable lookup. If this is a change (and not a reformatting artifact of moving things etc), then this would almost surely regress performance for arrays.

- Convert SumF function into a generic function

666f1fa

- Add new managed types (sbyte, byte, short, ushort, int, uint, long, ulong) - Add tests for some of the new types - Add new Benchmark tests

Smurf-IV added 2 commits July 31, 2019 09:41

SumF for an Array

cf405ac

- Make sure publics have Intellisense - Add Nullable types - Perform template on source selectors

- Split the Sum types out

4eb6b16

- Apply template to Spans<t> - Apply Template to List<T>

Smurf-IV changed the title ~~- Convert SumF function into a generic function~~ - Convert SumF functions for Array, Span, and List into a generic function Jul 31, 2019

Smurf-IV added 3 commits July 31, 2019 11:33

- Fix float selector special case

71459b2

- Add List benchmarks

[Enhancement] Can these API's also work on IList<T> jackmott#15

8ee3849

- Add SumF API's to redirect IList's to their base types

- Add Nullable types for List and IList

cb6ce9d

- Use "IList fast redirect" to implement all average Base and Nullabl…

d6a2542

…e types

- Complete base types applicable to Span<T>

3ecde38

- Add inlining hint to AverageIList

Smurf-IV added 5 commits August 1, 2019 10:08

Fix jackmott#21: [Bug] Span SumF is slower than a for loop over a span

e93f373

- Fix code to make it appear "As fast" Via the use of the "Coming out faster" acum = accum + nextVal method - Add ReadOnlySpan to `SumF` api's

[Enhancement] Can these API's also work on IList<T> jackmott#15

fed5794

- Done for Aggregate - Add All Span types - Add ReadOnlySpan - Reduce code in SumList

[Enhancement] Can these API's also work on IList<T> jackmott#15

d3b706e

- Fix copy paste error - Add better intellisense text

Investigate jackmott#22: [Question] Span_FirstF and List_First are sl…

1e464b9

…ower than just calling foreach -> Why?

Investigate jackmott#22: [Question] Span_FirstF and List_FirstF are s…

81c2a40

…lower than just calling foreach -> Why?

Smurf-IV mentioned this pull request Aug 3, 2019

[Bug] using SumS on a ushort[] throws System.InvalidCastException #18

Open

Smurf-IV added 6 commits August 5, 2019 09:27

Investigate jackmott#15: [Enhancement] Can these API's also work on I…

b762eff

…ReadOnlyList<T> - Change over to IReadOnlyList

More for jackmott#15: [Enhancement] Can these API's also work on IRea…

ac38801

…dOnlyList<T> - Implement LastF's the same way as FirstF's

Initial commit for jackmott#24: [Enhancement] DefaultIfEmptyF should …

57932a1

…be implemented - Add Start File

MaxF's for jackmott#15 : [Enhancement] Can these API's also work on I…

c7652e0

…ReadOnlyList<T> - Use types instead of vars - Breakout Max Benchmarks - Breakout MaxF API's for different types into seperate files

MinF's for jackmott#15 : [Enhancement] Can these API's also work on I…

54e72fa

…ReadOnlyList<T> - Split MinF's out into seperate files - Change to NameOf for Selector parameter

WhereSumF's for jackmott#15 : [Enhancement] Can these API's also work…

c3669bf

… on IReadOnlyList<T> - Replacement to use nameof for source - Split more benchmarks out into seperate relevant files

- Note: Lists can have items added and removed whilst these API's are…

ce954e2

… in use - The IReadOnlyList<T> represents a list in which the _number_ and _order_ of list elements is read-only.

Smurf-IV added 2 commits August 7, 2019 14:39

Invesigate jackmott#25: [Enhancement] Please also target .net4.8 and …

3bafae3

…Benchmark - Add Ratio Comparisons to Benchmarks - Add .Net4.8 Output directories - Explicit call types for Average

Investigate jackmott#25: [Enhancement] Please also target .net4.8 and…

9c9dd9d

… Benchmark - Attempt to sort why Appveyor is unhappy. - Sort out the ratio detection code when all are put back in.

Smurf-IV added 8 commits August 8, 2019 10:39

- Attempt to fic AppVeyor

25c68fc

- Another Attempt to satisfy Appveyor

064760c

- Appveyor again - Reset the bindingRedirects

66e84c4

- Look Apveyor Use the correct system.memry version will ya !

1963489

- (Maybe) Seems like the App.config is being ignored, so be explicit …

37f256c

…in the csproj file...

- Fix weird "Sometimes failing" test that show aggressive inlining do…

5927bcb

…ing weird stuff.

- test the unit Test Again !

5db00ad

ndrwrbgs suggested changes Aug 10, 2019

View reviewed changes

BrunoZell mentioned this pull request Nov 28, 2019

Add support for Span<T> #8

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

- Convert SumF functions for Array, Span, and List into a generic function #20

- Convert SumF functions for Array, Span, and List into a generic function #20

Smurf-IV commented Jul 30, 2019

Smurf-IV commented Jul 30, 2019

jackmott commented Jul 31, 2019

Smurf-IV commented Jul 31, 2019

jackmott commented Jul 31, 2019

Smurf-IV commented Aug 6, 2019

Smurf-IV commented Aug 6, 2019

jackmott commented Aug 6, 2019

Smurf-IV commented Aug 6, 2019

Smurf-IV commented Aug 7, 2019

ndrwrbgs left a comment

ndrwrbgs Aug 10, 2019

ndrwrbgs Aug 10, 2019

ndrwrbgs Aug 10, 2019

ndrwrbgs Aug 10, 2019

- Convert SumF functions for Array, Span, and List into a generic function #20

Are you sure you want to change the base?

- Convert SumF functions for Array, Span, and List into a generic function #20

Conversation

Smurf-IV commented Jul 30, 2019

Smurf-IV commented Jul 30, 2019

jackmott commented Jul 31, 2019

Smurf-IV commented Jul 31, 2019

jackmott commented Jul 31, 2019

Smurf-IV commented Aug 6, 2019

Smurf-IV commented Aug 6, 2019

jackmott commented Aug 6, 2019

Smurf-IV commented Aug 6, 2019

Smurf-IV commented Aug 7, 2019

ndrwrbgs left a comment

Choose a reason for hiding this comment

ndrwrbgs Aug 10, 2019

Choose a reason for hiding this comment

ndrwrbgs Aug 10, 2019

Choose a reason for hiding this comment

ndrwrbgs Aug 10, 2019

Choose a reason for hiding this comment

ndrwrbgs Aug 10, 2019

Choose a reason for hiding this comment