Boxing interface usage is allocating unnecessary heap data

[mgravell]

scenario: using the async API but fully sync in reality after checking completion; each call issues an InternalFasterSession boxed on the heap;

You can see this by running a memory profiler on this branch (branch: faster-boxing; project: /test/Benchmarks) in DEBUG mode (release mode runs BDN; you don't want that). The test runs 25,000 operations in a loop; the only heap alloc is:

Type Allocations
| - FASTER.core.ClientSession<FASTER.core.SpanByte, FASTER.core.SpanByte, Input, Output, FASTER.core.Empty, CacheFunctions>.InternalFasterSession 25,000

Since InternalFasterSession is a struct, this is unlikely to be intentional.

This is almost certainly where InternalFasterSession is being passed as a IFasterSession<Key, Value, Input, Output, Context> - for example DoSlowOperation, AsyncOperationInternal, etc, which will force it to be boxed. To avoid boxing, the type must be either:

• typed as itself, InternalFasterSession
• "constrained", i.e. passed (and used as) a generic TSession where TSession : IFasterSession<Key, Value, Input, Output, Context> (used in a few places, such as FasterSession in RmwAsync)

but: any time it is assigned / passed as IFasterSession<...>: it will box, i.e. allocate

[badrishc]

Your findng is accurate, but the reason we have not addressed it is because there is a standard pattern we use to avoid this:

var status = sync-call
if (status.IsPending)
   (await or sync-wait) complete-pending

An example for Read along these lines is in Garnet here: https://github.com/microsoft/garnet/blob/main/libs/server/Storage/Session/MainStore/MainStoreOps.cs#L18

[mgravell]

To confirm, then: using the synchronous API, checking IsPending, and in the pending scenario using asynchronous completion: is equivalent (functionally) to using the asynchronous API in the first instance? (give or take having to wire up cancellation etc, which I can probably do)

…

On Sun, 31 Mar 2024, 21:11 Badrish Chandramouli, ***@***.***> wrote: Your findng is accurate, but the reason we have not addressed it is because there is a standard pattern we use to avoid this: var status = sync-call if (status.IsPending) (await or sync-wait) complete-pending An example for Read along these lines is in Garnet here: https://github.com/microsoft/garnet/blob/main/libs/server/Storage/Session/MainStore/MainStoreOps.cs#L18 — Reply to this email directly, view it on GitHub <#907 (comment)> or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAEHMF3ZI6MRK7L7B33CBDY3BUW7BFKMF2HI4TJMJ2XIZLTSOBKK5TBNR2WLJDUOJ2WLJDOMFWWLO3UNBZGKYLEL5YGC4TUNFRWS4DBNZ2F6YLDORUXM2LUPGBKK5TBNR2WLJDUOJ2WLJDOMFWWLLTXMF2GG2C7MFRXI2LWNF2HTAVFOZQWY5LFUVUXG43VMWSG4YLNMWVXI2DSMVQWIX3UPFYGLLDTOVRGUZLDORPXI6LQMWWES43TOVSUG33NNVSW45FGORXXA2LDOOJIFJDUPFYGLKTSMVYG643JORXXE6NFOZQWY5LFVEYTIMZSGUZTAMRQQKSHI6LQMWSWS43TOVS2K5TBNR2WLKRSGIYTIMBSGQ2DKMVHORZGSZ3HMVZKMY3SMVQXIZI> . You are receiving this email because you authored the thread. Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub> .

[mgravell]

@badrishc I've spiked a switchover to the synchronous API exclusively, with any mention of async only in the (rare) IsPending scenario; I'll run the numbers when my machine is working