DokanFileInfo should it really be a class?

[TrabacchinLuigi]

in dokan net DokanFileInfo is a class which makes me think it lives in the memoryHeap, but maybe it lives in the stack, so it would make more sense if it was a struct, and we could decide to pass by reference or copy it around...
This is again a question because i'm not really sure how instances are marshalled between the kernel and the userland

[LTRData]

It gives quite a bit of performance gain to make it a struct, particularly in use cases with lots of file I/O. In my "high-performance" fork I have done exactly that and also changed most list buffers to enumerating results etc. But it involves several breaking changes and I would say that it is difficult to implement at this time in Dokan.net.

Link to the fork, if it sounds interesting:
https://github.com/LTRData/dokan-dotnet

I think though that some other things from this fork can easily be migrated back into the original dokan.net, for example the way it uses Span<byte> to avoid temporary byte arrays for Stream backends in .NET Core etc. (But I will make a separate post about that in some future.)

[TrabacchinLuigi]

that is quite intresting, as we are observing a lot of gc pressure using dokan

[TrabacchinLuigi]

i was looking at your fork, and i was thinking about why not using the in keywork instead of ref, it should be "safer", also i would pass all the other params with the in keyword

[LTRData]

i was looking at your fork, and i was thinking about why not using the in keywork instead of ref, it should be "safer", also i would pass all the other params with the in keyword

The in keyword is really only relevant for value types, so it would only be confusing to use it for other parameters.

[TrabacchinLuigi]

what about removing the object from the dokanfileinfo and adding an out parameter in createfile ? that would make sense, except for the added api change

[LTRData]

what about removing the object from the dokanfileinfo and adding an out parameter in createfile ? that would make sense, except for the added api change

Not sure I understand, the object needs to be marshalled by the DokanFileInfo.Context property logic and the GC pointer needs to be stored in a field in DokanFileInfo, so I think it makes sense to keep it a property in DokanFileInfo. Or did you mean something else?

[TrabacchinLuigi]

doesn't matter i was proposing a change in dokany

[Liryna]

I am fine with performance improvement that breaks the compatibility 👍

[TrabacchinLuigi]

another change we could try out to increment performance and reduce marshalling is create two structures for each method, instead of having let's say

public delegate NtStatus ZwCreateFileDelegate(
          [MarshalAs(UnmanagedType.LPWStr)] string rawFileName,
          IntPtr securityContext,
          uint rawDesiredAccess,
          uint rawFileAttributes,
          uint rawShareAccess,
          uint rawCreateDisposition,
          uint rawCreateOptions,
          [MarshalAs(UnmanagedType.LPStruct), In, Out] DokanFileInfo dokanFileInfo);

we could have

public delegate CreateFileResult CreateFileDelegate(in CreateFileInput input)
this would also increase readability
@LTRData you seem quite expert about those kind of changes

[LTRData]

The parameters to ZwCreateFile etc need to be the way they are to maintain compatibility with the C++ dll. Also, it would not give any particular performance gain to wrap it in a structure either because it just moves necessary marshalling to another place, when marshalling the structure instead.

The only thing that really needs marshalling in my fork with struct version of DokanFileInfo is the string with the file name and there is not much we can do about that. Everything else is just mapped values.

[TrabacchinLuigi]

i was proposing changin the C++ dll aswell

[LTRData]

Okay, but really, there is really no performance difference between marshalling fields in a structure compared to parameters. It is basically the same thing.

[Liryna]

@TrabacchinLuigi the V2 is a partial merge of a pr that is still opened on dokany repository and it is addressing this of have a single struct pointer that has all of the parameters in it.
I must they that I really liked the idea but in reality have all the wrappers and existing implementation to migrate to it is ... difficult. Like I am afraid the just loose people.

Regarding marshalling that another subject :D

[LTRData]

If we are talking about native C++, in my opinion the idea of allocating a struct on stack and send a pointer to it in function calls made a lot of sense on x86 architecture. However, on x64 architecture parameters are passed in CPU registers and do not require stack writes or reads, but a struct would need stack writes and then sending a pointer to that struct in a CPU register and the called functions need stack reads to get the values instead of having them in registers directly from start. The performance difference is extremely small, but in most cases it gives better performance to send parameters directly instead of wrapping them in a struct and send a pointer to the struct.

Then on to managed code and marshalling, any needed marshalling is basically the same for fields in structs and for parameters so there is no real performance gain for using one over another.

[TrabacchinLuigi]

So: + readability, -lot of work (double minus because i won't have much allocated time), -users would need to migrate projects. -would probably conflict with some already started work.
I think i'll skip that. but thanks to have took the time to reason about it with me