SkiaSharp icon indicating copy to clipboard operation
SkiaSharp copied to clipboard

add SKNativeObject allocation logging

Open mgood7123 opened this issue 1 year ago • 1 comments

Description of Change

can be toggled on/off via SKNativeObject.LOG_ALLOCATIONS (default is false)

Bugs Fixed

  • Fixes #

API Changes

None.

Behavioral Changes

None.

Required skia PR

https://github.com/mono/SkiaSharp/pull/2157

PR Checklist

  • [ ] Has tests (if omitted, state reason in description)
  • [ ] Rebased on top of main at time of PR
  • [ ] Merged related skia PRs
  • [ ] Changes adhere to coding standard
  • [ ] Updated documentation

mgood7123 avatar Jul 07 '22 14:07 mgood7123

Thanks for this PR. We should add some logging, but this must be a Debug-only feature, internal and opt-in by setting some property.

If you have a look at the way I set up THROW_OBJECT_EXCEPTIONS. It is a define that was added in debug builds:

* https://github.com/mono/SkiaSharp/blob/cb424e5230471ed300319e97cc7cb4fd9f771e03/source/SkiaSharp.Build.props#L88

* https://github.com/mono/SkiaSharp/blob/e603133c3548bbd3835cf1012eac80ce5640f209/binding/Binding/HandleDictionary.cs#L16

The reason for internal is to avoid any accidental APIS leaking, and then when it is in the conditions, we can do checks with and without the logging - maybe for perf or to determine things are working correctly for tests.

hmmmm i still like that it is toggle-able and public, as it may help end users track down allocations that they forget to dispose (which will add memory pressure until GC occurs)

making it togglable via building skiasharp itself (and via debug build) would likely be combersome since the end user will likely be debug building their app and not expect to build a debug build of skiasharp just to enable allocation logging

maybe we could offer both a debug nuget and a release nuget so that the end user does not need to manually build skiasharp as debug?

this way, we still keep the API's but only expose them in the debug build of skiasharp (assumable via #if DEBUG)

mgood7123 avatar Aug 17 '22 02:08 mgood7123

Closing this one as I am not sure this is the best way. Some more design/discussion/research needs to be done to make use of the tracing features in the .NET SDK: https://github.com/mono/SkiaSharp/issues/2777

mattleibow avatar Mar 02 '24 13:03 mattleibow