sharpdx / SharpDX

SharpDX GitHub Repository
http://sharpdx.org
MIT License
1.69k stars 638 forks source link

Proposal: `Try` APIs which allow HResult to be returned without throwing an exception #804

Open prasannavl opened 7 years ago

prasannavl commented 7 years ago

SharpDX currently, almost always uses the CheckError method to ensure the HRESULT returned by COM is ok. Otherwise it simply throws an exception. While its OKAY to do this in many scenarios, there are scenarios where the cost of throwing and catching the exceptions are completely un-necessary. It especially doesn't justify the inefficiency when you have codepaths that handes all HRESULTs.

However, since redoing the APIs itself to accommodate this will end up with breaking changes, and in practice its actually OKAY for many APIs to simply just throw an exception, a neat alternative would be to add Try methods that return the HRESULT directly to some of the APIs where it makes sense.

For example the EndDraw methods.

xoofx commented 7 years ago

Why not, if you have some APIs in mind like EndDraw , I will accept PR for this.

prasannavl commented 7 years ago

@xoofx, the ideal candidates I have in mind are SwapChain's Present, D2D like EndDraw and some CreateDeviceXX methods.

However, while some of them are checked only in the manually rewritten classes, most of them like the D2D EndDraw for example does the result check in the generated code.

On first look through the codebase, it seems they are all done in the T4 here: https://github.com/sharpdx/SharpDX/blob/master/Source/Tools/SharpGen/Templates/Method.tt#L373

I'm wondering if there are hooks into this code generation? Or would it require deeper changes into the SharpDX code generation? It would be great if you could point me in the right direction.

Also, on second thought, I'm a little conflicted on Try methods versus using the same methods with perhaps an overload that take the ResultDescriptor as an out parameter.. The latter would be lighter on the classes being not too polluted with many methods, and also since the Try in the .NET ecosystem generally uses the actual result as the out, and not the result status code. So, I'm not sure using a non-idiomatic one is the best approach.

PS: As an added advantage, the latter approach can also be worked out to do this to potentially every single method that returns a HResult. This way the existing code path is entirely untouched, and will work as expected thworing exceptions, however for things where direct result is needed, developers can dig in without any exceptions being thrown, since every method will have this overload as well.

xoofx commented 7 years ago

This has been already used in many places (check for example this) Basically you mark the method as private, rename it the mapping as well (by adding a name with an underscore for example, check further down in the file)

Then you can provide the same signature which does the check and another one with Try that returns directly the Result (or if you don't need to perform anything else in the Try, just let the method public and rename it to TryXXX and reuse TryXXX from the XXX method)

mrvux commented 7 years ago

I remember having to do that a while ago (used a private build), since I needed to check for occluded (also needed to avoid exception on Frame Statistics).

Here is a commit for Present: https://github.com/mrvux/SharpDX/commit/0d40bd3c9f4e787720f4cf89689521bbc982d62c

And added it for Frame Statistics too: https://github.com/mrvux/SharpDX/commit/963bac4746a59c24deede8e5583c36f857e6928a

There's also EndDraw in the branch.

If you need another one let me know, same if you ok for PR.