Enhancement - Transition from Lists to ConcurrentDictionaries

[jmscraig]

rom all the reading multiple threads even reading the same list with "contains" or even a .ToArray() call is like buying a lottery ticket.. best case it will happen rarely but eventual some hits

Over time (hopefully sooner than later) I recommend the replacement of Lists used outside a single thread be replaced with ConcurrentDictionaries.

ConcurrentDictionaries 200-400% slower to load each row than a list, but provide lock like security for read with no identifiable locks or locking if your the reader.

We read so much more than we write so the ConDict might be a massive win.

fyi .. .example use of ConcurrentDictionary in OnMarketData()

`

uint ui_volume ; lock (e.Instrument.SyncMarketData) { ui_volume = (uint)e.Volume ; _cdictionary.AddOrUpdate(e.Price, ui_volume, (price, oldVolume) => oldVolume + ui_volume); }

`

NT Jim posted a good example use of Concurrent dictionary more complex than replacing the lists.. Using a data Class to add multiple items per dictionary row.

https://ninjatrader.com/support/forum/forum/ninjatrader-8/strategy-development/102909-imbalance-strategy?p=803902#post803902

For as fast as we add and remove orders manage order details in classes stuffed into dictionary would slow things down a lot and generates a lot heap GC pressure so not encouraged unless really needed but proves the ConDict is viable for far more than is needed here.

Rather, use of a number of ConcurrentDictionaries(, ) very similar as to how we use lists today would be very fast and simple to write and maintain.

---

just replacing the list looks much simpler than NT Jim's example..

much more like example from dotnetpearls

using System; using System.Collections.Concurrent;

class Program { static void Main() { // New instance. var con = new ConcurrentDictionary<string, int>(); con.TryAdd("cat", 1); con.TryAdd("dog", 2);

    // Try to update if value is 4 (this fails).
    con.TryUpdate("cat", 200, 4);

    // Try to update if value is 1 (this works).
    con.TryUpdate("cat", 100, 1);

    // Write new value.
    Console.WriteLine(con["cat"]);
}

}

---

Manage a simple ConDict(, ) for StopLoss Orders

private ConcurrentDictionary<string, Orders> activeOrders; private ConcurrentDictionary<string, Orders>entryOrders; private ConcurrentDictionary<string, Orders> stopLossOrders; private ConcurrentDictionary<string, Orders> profitTargetOrders;

else if (State == State.DataLoaded) { stopLossOrders = new ConcurrentDictionary<<string, Order>>(); }

OBU(), OMD(),.. stopLossOrders.TryAdd("Unique Stp order name", "Working");

---

// Contains is slower, TryGetValue is awesome if (!stopLossOrders.ContainsKey("Unique Stp order name")) { return;} }

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

---

protected void CancelStopLossOrders(bool return) { if(stopLossOrders.Count > 0) { // No Lock needed... it is a ConcurrentDictionary. Yea! foreach (Order o stopLossOrders.Keys) { If( IsOrderActiveCanCancel(o) )
CancelOrder(o); stopLossOrders.Clear(); } return true or false for success or fail ; }

---

Order o; stopLossOrderStatus.TryRemove("Unique Stp order name", out o )

TryRemove(searchKey, out CityInfo retrievedValue) stopLossOrders.Clear()

---

a wee bit later... use ConDicts to provide Scalability for people to many low maintenance trading plans ...

// In this use case where the updates are less frequent use of a data class in a Dict is great.

public class StopLosslProftiTargetExitPlans { public double stopLoss1Distance = 0.0; public double profitTarget1Distance = 0.0; etc. }

private ConcurrentDictionary<Key(int or string), StopLosslProftiTargetExitPlans> exitPlans;

else if (State == State.DataLoaded) { exitPlans = new ConcurrentDictionary<Key(int or string), StopLosslProftiTargetExitPlans>(); }

[MicroTrendsTom]

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 6) 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

[MicroTrendsTom]

"a wee bit later... use ConDicts to provide Scalability for people to many low maintenance trading plans ..." See Backlog https://github.com/MicroTrendsLtd/NinjaTrader8/issues/31#issue-759996352 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

[MicroTrendsTom]

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

[MicroTrendsTom]

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 :-)

[jmscraig]

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!

[jmscraig]

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;

[MicroTrendsTom]

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: https://github.com/MicroTrendsLtd/NinjaTrader8/issues/31#issue-759996352

[MicroTrendsTom]

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

[MicroTrendsTom]

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 👍

[jmscraig]

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 }