MOB462 Expose marginUsage from isolated positions #322
Conversation
johnqh
requested review from
rosepuppy,
aforaleka,
jaredvu,
moo-onthelawn,
ruixhuang,
mike-dydx and
prashanDYDX
| @@ -400,6 +406,7 @@ data class SubaccountPosition( | |||
| existing.resources !== resources || | |||
| existing.childSubaccountNumber !== childSubaccountNumber || | |||
| existing.freeCollateral !== freeCollateral || | |||
| existing.marginUsage !== marginUsage || | |||
There was a problem hiding this comment.
Why are we using referential equality here?
Also taking a step back, what is the goal of this if block? Why does returning existing vs a new object matter? Do we need to check fields one by one? Why not just construct the object and use built-in data class equality?
There was a problem hiding this comment.
You are right, childSubaccountNumber is not an object so shouldn't be referential equality. Changed.
All others are objects.
Those output objects can be larger hierarchal objects. Creating data object from IMap will be slower since it is not just a single object being created. It is important for abacus to be as fast as possible as we are dealing with three platforms.
When existing object (which enables referential equality) is returned, downstream code uses referential equality to check if additional code gets triggered. Again, this gives us much better performance than returning a data object even if there is no change.
There was a problem hiding this comment.
i see, i'm curious if this architecture was adopted in response to actual perf problems or if it's more theoretical? it is not a common pattern and sort of fragile with new devs onboarding, i could easily see someone accidentally creating new objects instead of checking against existing.
Simple PR: Design shows the margin usage changes for the isolated position. That needs to be exposed in object.