Enhancement - Transition from Lists to ConcurrentDictionaries about ninjatrader8 HOT 10 CLOSED

i have added public bool IsOrdeActiveConcDict
[XmlIgnore]
[Browsable(false)]
public ConcurrentDictionary<long, Order> OrdersActiveConcDict { get; set; }

just an FYI..

I read that public objects have additional layers of referencing for each call.. more complexity and slower.

the reference here recommends the public properties does not host the list but calls an internal list. Internal calls call the internal list. Oh, .. might have lost that URL.

Found a few references..
https://jira.sonarsource.com/browse/RSPEC-2365
https://rules.sonarsource.com/csharp/tag/performance/RSPEC-2365
https://rikkeisoft.github.io/sonar-rules/cs.html

Most developers expect property access to be as efficient as field access. However, if a property returns a copy of an array or collection, it will be much slower than simple field access, contrary to the caller's likely expectations. Therefore, such properties should be refactored into methods so that callers are not surprised by the unexpectedly poor performance.

Compliant Solution
private List _foo = new List { "a", "b", "c" };
private string[] _bar = new string[] { "a", "b", "c" };

public IEnumerable GetFoo()
{
return _foo.ToList();
}

If so public IsOrdeActiveConcDict might look something like

private ConcurrentDictionary<string, Orders> ordersActiveConcDict;

[XmlIgnore]
[Browsable(false)]
public IEnumerable<string, Order> OrdersActiveConcDict()
{
return ordersActiveConcDict();
}

• I have low confidence this bit is accurate "IEnumerable<string, Order>".

The easier path to start faster to Go//No Go test / assess concept viability and reliability might just be to start with the internal Dict.
private ConcurrentDictionary<string, Orders> ordersActiveConcDict;

from ninjatrader8.

What we proved over the last 10 days was, list locking is not required generally speaking unless we are going to enumerate a list - while its been adding to. And if we do we can use finegrain techniques etc and you bool example is a good one.

things to consider
The engine is migrated not new - albeit not a direct copy an evolution and simplification for a specific task to power 4 bracket systems no compounding etc - the former had no errors in 10 years or deadlocks with 1000s of users in NT7 but this is NT8 and we saw so many deadlocks...
NT8 is M threaded so we do have to understand the flow and if a lock is needed at all.
It is better to have no locks if we can as they can cause more problems than they solve
Some of the locks put in place caused deadlocks due to the flow and events also firing on other threads hitting a lock on the same object and so forth - over lists and were not required for the majority as we dont mind if the list changed -
Events are synchronous to an object flow such as order but the flow of events is async to a different object scope so locking is not required for the majority of items.

Solution Implemented thus far

1. Locks only where it is certainly required.
   2)List "dirty reads" are fine -
   3)with lists - the main problem to avoid is enumeration while a possible change to the list can occur
   4)Logical flow aware reads and usage are also key - in a flow where no change will occur etc -
   5)Use ToArray ByVal copies for enumerations or for linq where/select/count etc - GC overhead etc
2. Be careful not to use some methods from a Strategy with NT8 keep in scope of the methods exposed in the strategy bvase class -

Warnings of NT8 and Concurrent objects
concurrent objects such as Q cause a no returning method call to simply add - due to NT8 threading model and was unreliable..
So currently no pressing technical reason to change it currently as we are past the deadlocks part "Touch wood" with a series of clunky techniques that might not be the best....

Conclusion this end

1. With a mass of high priority items to tackle there is no time to R&D the pros and cons of it etc -
2. Would we see a better route than 4 above for example.. .can you enumerate a changing dictionary or pull linq selects off it? - YES
3. based on intrinsic concurrent class failures - will Concurrent dict even work? or only for Realtime threading - To be proved -NT Jim thinks so
   4)if there was time to do this yes.... it would be great to see this :-)

Conclusion of the conclusion: Yes Im in favor to R&D and test it out and compare one versus the other :-)
even if there is a full way or half way solution etc

Game Plan Ideas

What game plan would make it an easy transition?

1. Leave the old list in place but add a ConcDict alongside with the Sufix ConcDict
   E.G. OrdersActiveConcDict
   Allow usage of list mode or dict mode via a public Property
   Easy low risk

2. High impact changes and lost more work -but a lot more fun tech wise
   Dictionary of Proposed Trade as structure to include a generic structure
   class TradeOperationGeneric
   {
   IEnumerable OrdersEntry
   IEnumerable OrdersExitStopsLoss
   IEnumerable OrderExitsTargets

bool IsOrderEntryAnyActive
bool IsOrderExitsStopLossAnyActive
bool IsOrderExitsTargetAnyActive
bool IsOrdersActve
}
or
3)
class TradeOperation
{
Order OrderEntry1
Order OrderEntryOcoLong
Order OrderEntryOcoShort

Order OrderStoploss1
Order OrderStoploss2
Order OrderStoploss3
Order OrderStoploss4

Order OrderTarget1
Order OrderTarget2
Order OrderTarget3
Order OrderTarget4
}

i think 1 dictionary which contains the structure for each trade is a potential but what of ad hoc user trades.
perhaps the class structure would contain a list of orders a list or targets... as opposed to a hard wired structure of 4 + 4 etc
4 +4 would be faster.

By the way this method allows for position compounding and more flexibility

But the changes are now not so simple and much broader.
So all of a sudden its a can of worms 👍 i'd prefer generics anyday ;-) but we know the short cut is hard wired

i think if we think in terms of conccurency
Ramblings about similar stuff, lets look at MSSQL server and reading from a table
If we set the Isloation level to dirty reads we dont lock up the system waiting for locks...and commits etc
This is no different we dont need to lock up lookup lists - unless its really really really neccessary -
we dont want to halt the events adding updating etc
we just have to be careful in enumerating them when changes can occur in parrellel...
sometimes we might put a lock if we really really really have to stop and read and release but a copy is alos fine

Lock or not to Lock
so there is a very limited reason to use locks in this engine and it should be very carefully implemented
Concurrent dicts - ok sure but what of the impact and effort etc -thats the only item
maybe there is a halfway house

There is better Trade Engine design
but then there are better trade engine workflows - but to finish a product is to know that and prevent "Creeping excellence" from preventing progress where striving for perfection as we know a better way half way through a current task -prevents ever completing -and the trick is to complete it

i can think of methods where the orders submission need not live in a list at all - its just legacy migrated
it can just a be moniker a sum or count and tally in - and then a tally out to say all is balanced,...
or a array of bools to say done not done etc
or local instance variable etc

so within context of this engine what is worth changing and why and what is the impact etc
i think some gradual introduction rather than a back to board and go forwards etc
So we might just live with imperfection in the knowledge there is better - and look to implement the higher value features first etc

Compromise

A small change and keep the old list in place - target 1 main list OrdersActive? = OrdersActiveDict
so on that basis to keep impact small if any at all - a small change and experiment to replace just 1 list to begin with rather than aim for 1 super dictionary etc and keep the list code in and select which mode to use so we can compare if there is any advantage at all for it etc

Larger changes
for a more massive reworking that would be another engine project version etc

So a small change and experiment as surely we are both itching to know the difference and abilities

from ninjatrader8.

"a wee bit later... use ConDicts to provide Scalability for people to many low maintenance trading plans ..."
See Backlog #31 (comment)
Yes that would be in a different layer derived from the base - at this level keep it generic and even move away from the 4 bracket limitation and even position compounding is easier etc above "class TradeOperationGeneric" or the static version

from ninjatrader8.

on that basis i have added this -

public bool IsOrdeActiveConcDict - to control if we use this or the list

[XmlIgnore]
[Browsable(false)]
public ConcurrentDictionary<long, Order> OrdersActiveConcDict { get; set; }

Instanstiate in Transition
used onOrderUpdate etc

if (OrdersActiveConcDict.TryAdd(order.Id,order))
and
if (OrdersActiveConcDict.TryRemove(order.Id, out orderRemoved))
so we need a method to test also to replace the enumeration byVal pattern for IsActiveOrdersExist etc
would that be CancelAllOrder by any chance :-)
there will be others too

from ninjatrader8.

This is way more fun than my job in hand will have to come back to this later on :-)
Soak test mode with the batch close for concurrentDict - no more .ToArray() ;-)

Initial testing shows its a go :-)

from ninjatrader8.

Warnings of NT8 and Concurrent objects

Yikes! And to add more. What may be the most difficult aspect of Multi-Threading is NT does not tell us what objects they are locking on under the hood so when we execute API calls. If we can not lock on the same it is just a matter of time before exception or deadlock. In my fastest strategies I see this with calls to CBI objects.

I figure its a good idea to follow NT's lead on use of ConDict. I have not yet found the original but these code was said to be against the pattern of how NT uses ConDict in SuperDom for adding a Volume column.

pattern

1. lock on the NT API element
2. broad use of normal ConDict capabilities
   '
   uint ui_volume ;
   lock (e.Instrument.SyncMarketData)
   {
   ui_volume = (uint)e.Volume ;
   _cdictionary.AddOrUpdate(e.Price, ui_volume, (price, oldVolume) => oldVolume + ui_volume);
   }
   '

"1) lock on the NT API element" is why I am liking the "out of band" "other thread" or soft lock approach I wrote for CancelAllOrders() approach to avoid slow downs, hangs and deadlocks and ensure the tick by tick order management.

i can think of methods where the orders submission need not live in a list at all

Agreed. But then we are wholly dependent on API calls to get order Names & Status, open order quantity quite frequently and .... (see note above "What may be.. ").

moniker a sum or count and tally in

I think there is power in tallying in and out to keep a short constant set of integers that describe ExpectedStrategyPosition
expectedStrategyPositionQtyLong
expectedStrategyPositionQtyShort
expectedStrategyPositionQtyStopLoss
expectedStrategyPositionQtyProfitTarget

Then for me it's

• A quick validation every 20-40 ticks or X seconds of and match between Position.Quantity as a quick low cost Quality Check
  and a test of these test these to make sure it looks like Position.Quantity is covered.

• Less Often (and in a seperate thread) obtain a lock on the CBI elements every X ticks or X seconds a query to download the last X (50-300) order update log entries and tally up open ProfitTarget Quantity and ProfitTarget Quantity to to see if that bit is a match to expectedStrategyPositions.

• When the strategy has been Flat for X seconds CancelAllOrders() and Flatten any open Positions.

A small change and keep the old list in place

Yes. Small and simple while leaving existing code there just commented for easy rollback. Stage in..

Initial testing shows its a go :-)

Awesome!

from ninjatrader8.

private ConcurrentDictionary<string, Orders> ordersActiveConcDict;

Excellent call i was in a hurry now we know it is a compatible object we can whitebox the code and optimise it etc
maybe a public field wont hurt if its needed - or encapsulate in methods dont expose etc
Why do we even need a public property

"I have low confidence this bit is accurate "IEnumerable<string, Order>"."
Yup i would say its not possible - Keys, Values would expect to Implement etc...

Added to backlog: #31 (comment)

from ninjatrader8.

I think there is power in tallying in and out to keep a short constant set of integers that describe ExpectedStrategyPosition
expectedStrategyPositionQtyLong
expectedStrategyPositionQtyShort
expectedStrategyPositionQtyStopLoss
expectedStrategyPositionQtyProfitTarget

This does depend a lot on the layer putting on the orders
A method to say OrdersSopLossWillSubmit is a pattern i have used and so should be easy to get a count at same time assuming the coder of the layer deriving from it all is diligent to set it correctly etc

Another pattern i use is the sum of Qty Open versus sum of quantity of Stoploss and we dont care for profit targets etc
As long as we can get that we are good etc - caveats are updates to fills as we measure etc

from ninjatrader8.

To me the only advantage for the concDict thus far is enumerations without an explicit copy
and if ever needed to not use the external lock statement etc
Assuming NT8 cant cope with that object type (as ConcQueue it couldnt) then its a winner provided the insert is not slower that a contains and add list op is quicker and the combo of byVal copy of processing on the list

Soif there are still caveats to ConDict as opposed to the usage of a list with the correct logic to avoid problems then lists are fine

We really dont care if a list changes as its read - within most contexts add/remove/contains
we only care about errors to do with changes during enumeration
the code is written with a flexible loose model to understand and cope with it all
the analogy SQL Server table dirty reads -dont wait for locks and commits etc

but if concDicts are better then the list implementation for overhead and ease of use to prevent issues...
and NT8 does not have a hidden allergic reaction to them then its all go go go - otherwise lists with careful use is fine- its looking good 👍

from ninjatrader8.

Good posts. All the whole you have had less trouble than I have with Lists in NT8 and that is a good thing.

ConDict is slower than a List at Contains but faster and better (you are ensured one unique value when a list can have dupes) when you use TryGetValue rather than Contains.

if (stopLossOrders.TryGetValue("Unique Stp order name", out Order))
{
do something
}

from ninjatrader8.