GC Free implementation of Enumerate

[TJHeuvel]

The existing implementation filled a list, and then returned an enumerator to that.

The list itself is heap allocated, any required resizing will also increase GC pressure.

The new implementation implements an enumerator directly on the underlying native Enumerate methods, preventing any GC from happening. The Hid api also returns the new HidEnumerator type directly rather than an interface to prevent boxing.

[badcel]

Thank you for the PR. I'm very interested in it 🚀

But bear with me that there will be a lot of questions partly technically and partly more general (which you don't need to answer if you don't want to).

Let's start with the non technical questions:

• Are you using HidApi.Net for a project / app or did you just stumble about it?
• If you use it for a project, does it perform well?

I ask this because it's hard to know how HidApi.Net performs under real world conditions. I have only a minimal pet project which is only using a portion of the available API.

Now the more technical questions:

• I think your PR is a breaking change? So what do you think about marking the existing code as obsolete and implementing your code as a new path forward? I don't want to break the API with new releases very often.
• Why did you implement the change? Did you experience problems with the current implementation?
• I don't understand which use case you mean with "resizing" the list? The underlying type is a list so should appending to it not be a problem? (I did not check the implementation of "Append" but would assume there is a special case for lists?).
• Why does the implementation does not implement "IEnumerable"?
• How do you handle the "Dispose" case in a real world scenario? Aren't there any possibilities to avoid the problem of forgetting to "Dispose" or is this handled by foreach automatically?
• Do you know other implementations which are implemented in this way (with the dispose scenario)?

[TJHeuvel]

1. I was looking to create my own version of StreamdeckSharp which would need a solid HID solution. I like that you wrap a C library, as an ex game developer i care about performance :)

2. None yet

3. I don't know enough about API design to recognise a breaking change. Anyone just doing foreach would not notice a difference, but ABI's surely change. If you think an obsolete is worth it i can add it. I do think i'm exposing a struct thats in a folder called Internal, is that an issue?

4. I judged the use of a List especially inefficient, i implemented this to optimise. Also i believe its a very good match for C#'s iterator design.

5. In .NET a List is implemented as an array with count. Once you add more elements than the array can contain, a new one is created and the items copied over. This resizing is especially inefficient and should be avoided. More info: https://steven-giesel.com/blogPost/06e2c8e8-0119-4e8d-9227-3cb922a8c916

6. This is somewhat common i believe. How C# implements the foreach-enumerator pattern predates generics, the compiler simply looks for MoveNext etc methods on the returned type. This is why its possible at all. My implementation uses a struct which can be stack allocated. When a more generic type, like IEnumerable, is returned the struct has to be boxed causing allocations. https://learn.microsoft.com/en-us/dotnet/csharp/programming-guide/types/boxing-and-unboxing

7. When using this construct in foreach the spec defines Dispose is called (https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/language-specification/statements#1395-the-foreach-statement). There is indeed extra risk in other useages not getting dispose called.

8. .NET builtin types do this too, for example List (https://github.com/dotnet/runtime/blob/1d1bf92fcf43aa6981804dc53c5174445069c9e4/src/libraries/System.Private.CoreLib/src/System/Collections/Generic/List.cs#L1185), Span https://github.com/dotnet/runtime/blob/1d1bf92fcf43aa6981804dc53c5174445069c9e4/src/libraries/System.Private.CoreLib/src/System/Span.cs#L230

[TJHeuvel]

(My apologies i'm packing for holiday and wont respond for a while)

[badcel]

No problem. Have a great time on your holidays 🗺️

Thanks for the detailed answers. Great you choose HidApi.Net for your project.

If you are into performance you are welcome to take a look at WCharT.Net. It's a library born from code that was originally part of HidApi.Net and allows to work with WCharT characters from C. Now it is a dependency as I did not find anything similar on nuget and thought it may be helpful for others.
Actually I'm not very happy with it. Neither from an API design point of view nor from the code itself. So any remarks are very welcome.

I wanted to separate the internal code from the public code to make it easy to see the public API on source code level. So everything in the internal folder is actually internal.

For me it is a breaking change because there could already be code that uses the result of Enumerate which is not used in a foreach.

Implementing IEnumerable<> shouldn't be a problem if it is used in a foreach as the GetEnumerator method should be preferred before boxing happens, if I understand the spec correctly?
Implementing the interface would allow users to benefit from extension methods which work on IEnumerable which is nice to have I think.

So if we keep IEnumerable the method signature keeps the same. In theory it would still be a breaking change because a user could have cast the IEnumerable to a List or similar things. But I don't know if this is very important for me. If someone mingles with internal stuff, things can break.

In List and Span implementations the enumerator is a public struct inside the enclosing type. So I would suggest we do the same and thus solve the issue with a public struct inside the "internal" folder.

Are you sure the Dispose method works like intended? It is at least a change in behavior like you implemented it: According to the HIDAPI docs for hid_enumerate: "The returned value by this function must to be freed by calling hid_free_enumeration(), when not needed anymore."

I think it can happen that you pass NULL or some element from the middle of the linked list and not the initially returned value of the hid_enumerate function.

One thing that just came to my mind: I'm not sure if the code works like intended if the GetEnumerator method is returning IEnumerable? The spec says "Member lookup is performed on E..." because the member lookup would happen on an interface.

It could perhaps not work as List is doing this:

public Enumerator GetEnumerator() => new Enumerator(this);

        IEnumerator<T> IEnumerable<T>.GetEnumerator() =>
            Count == 0 ? SZGenericArrayEnumerator<T>.Empty :
            GetEnumerator();

        IEnumerator IEnumerable.GetEnumerator() => ((IEnumerable<T>)this).GetEnumerator();

So I wonder what would be a good way forward? The Enumerator could implement IEnumerable similar to List and GetEnumerator would still return a Enumerator?

What do you think?

Or we create some new class just for enumerating and let the old code be the old code and mark the Hid.Enumerate method as obsolete?

[badcel]

Are you still planing to pick this up again?

[badcel]

Closed in favour of #122.