Go Back   NinjaTrader Support Forum > NinjaTrader 8 > Indicator Development

Indicator Development Support for the development of custom indicators using NinjaScript.

NinjaTrader
Reply
 
Thread Tools Display Modes
Old 01-31-2017, 06:28 AM   #1
tradesmart
Senior Member
 
Join Date: Nov 2010
Posts: 267
Thanks: 122
Thanked 108 times in 80 posts
Default Collection was modified

Periodically, I encounter a "Collection was modified; enumeration operation may not execute" error. What would be the recommended procedure to avoid such errors? I'm trying today to wrap some code sections within lock, as shown below.

I have code in OnMarketData that populates a dictionary of dictionaries for use in OnRender, which scans each dictionary.

OnMarketData(MarketDataEventArgs e)
Code:
cdict_barvolume = new Dictionary<double, VolumeInfoItem>();
if (!cdict_barvolumedictionaries.ContainsKey(CurrentBar))
   cdict_barvolumedictionaries.Add(CurrentBar, cdict_barvolume);

lock (e.Instrument.SyncMarketData) // will this be helpful?
{
   if (!cdict_barvolume.TryGetValue(e.Price), out vii_value))
      cdict_barvolume.Add(e.Price), new VolumeInfoItem());
   vii_value = cdict_barvolume[e.Price];
   /// ... etc.
}
then, in OnRender(ChartControl chartControl, ChartScale chartScale)
Code:
lock (cdict_barvolumedictionaries) // will this be helpful?
{
   for (int index = ChartBars.FromIndex; index <= ChartBars.ToIndex; index++) 
    {
      lock (cdict_barvolumedictionaries[index]) // will this be helpful?
      {
         // ... draw some objects using the dictionary's data
      }
    }
}
tradesmart is offline  
Reply With Quote
Old 01-31-2017, 09:39 AM   #2
NinjaTrader_Jesse
NinjaTrader Customer Service
 
NinjaTrader_Jesse's Avatar
 
Join Date: Mar 2014
Location: Denver, CO
Posts: 5,065
Thanks: 30
Thanked 1,291 times in 1,209 posts
Default

Hello tradesmart,

I looked at the provided syntax but nothing directly sticks out as the cause for the error. In general the error is caused when you are iterating a collection and modifying it at the same time. for example if you were using a foreach loop on a list, and removing an item or adding an item to that list in the loop. In this specific example a for loop may be able to be used instead.

With the syntax you provided, have you tested by commenting the logic out to ensure this is the specific logic causing the error? If not, I would suggest to try and locate the specific line of syntax that generates the error, this would provide more detail on the reason. As noted earlier in the example, this is often caused by trying to do an action to the collection you are iterating over.

The only item I can see that may cause a problem would be in the first example where you are adding items to a collection, potentially if this logic was contained in a loop of the same collection. This would just be a guess based on the limited scope I can see currently. If you could provide a more complete example of the specific logic throwing the error, it may be more apparent.

I look forward to being of further assistance.
NinjaTrader_Jesse is online now  
Reply With Quote
The following user says thank you to NinjaTrader_Jesse for this post:
Old 01-31-2017, 11:38 AM   #3
tradesmart
Senior Member
 
Join Date: Nov 2010
Posts: 267
Thanks: 122
Thanked 108 times in 80 posts
Default

Unfortunately, this is one of those intermittent errors that is not readily reproducible.

Today, after adding the lock statements I boldfaced in the sample code, the "collection was modified" error has not returned; the relevant workspace has been running for over 4 hours now without a hitch.

I did get an "Unable to write cache data" error after closing another workspace to load the one where I'm stress-testing for the "Collection was modified" error, but it did not appear to affect anything adversely.

Quote:
2017-01-31 07:27:29:103 Cbi.Instrument.RequestBars (to Provider): instrument='ES 03-17' from='1/31/2017 7:00:00 AM' to='1/31/2017 11:59:59 PM' period='1 Tick'

2017-01-31 07:27:29:127 ERROR: Data.Bars.Load7: System.IO.IOException: The process cannot access the file 'C:\Users\G\Documents\NinjaTrader 8\db\cache\CME US Index Futures ETH.Central Standard Time\MINUTE\ES 03-17\Minute_1_1_Last_Close_Tick_Minute_1.Last.ntb' because it is being used by another process.
at System.IO.__Error.WinIOError(Int32 errorCode, String maybeFullPath)
at System.IO.File.InternalDelete(String path, Boolean checkHost)
at NinjaTrader.Data.BarsSeries.Load()
tradesmart is offline  
Reply With Quote
Old 01-31-2017, 12:15 PM   #4
NinjaTrader_Jesse
NinjaTrader Customer Service
 
NinjaTrader_Jesse's Avatar
 
Join Date: Mar 2014
Location: Denver, CO
Posts: 5,065
Thanks: 30
Thanked 1,291 times in 1,209 posts
Default

Hello tradesmart,

I am unsure the new error message would be related to the one you had commented on originally, this could be something else in the workspace and just happens to be in the same timeframe. Regarding the original item, because this is intermittent I suppose only time will tell if that was the solution to the problem. You would need to test for the period it usually takes to see if the error comes back.

If you are running into a situation where threading is causing a problem I.E. you are modifying a collection at the same time another thread is, potentially the lock has corrected the problem as this is its purpose.

I look forward to being of further assistance.
NinjaTrader_Jesse is online now  
Reply With Quote
Old 01-31-2017, 12:19 PM   #5
tradesmart
Senior Member
 
Join Date: Nov 2010
Posts: 267
Thanks: 122
Thanked 108 times in 80 posts
Default

Quote:
Originally Posted by tradesmart View Post
Unfortunately, this is one of those intermittent errors that is not readily reproducible. ...the relevant workspace has been running for over 4 hours now without a hitch.
I spoke too soon. Another instance of the error arose a half hour later.
Quote:
11:58:42 TROUBLE! gLiquidity.OnRender Collection was modified; enumeration operation may not execute.
Source: mscorlib
StackTrace: at System.Collections.Generic.Dictionary`2.Enumerator .MoveNext()
at NinjaTrader.NinjaScript.Indicators.gws.gLiquidity. OnRender(ChartControl chartControl, ChartScale chartScale)
TargetSite: Boolean MoveNext()
ChartBars: (Base 1-Tick) SecondBarsType (Second: 6/4) TickReplay OnPriceChange
Here's the OnRender() code; the dictionaries are populated in OnMarketDepth(). I don't see anything that is attempting to modify the dictionaries; it's merely reading them.
Code:
		protected override void OnRender(ChartControl chartControl, ChartScale chartScale)
		{
			try
			{
				base.OnRender(chartControl, chartScale); // plot 
				
				if (Bars == null || Bars.Instrument == null || IsInHitTest 
					|| ChartBars == null || chartControl == null || chartScale == null)
					return;
		
				SharpDX.Direct2D1.Brush brushDXhilite	= ColorHighlight.ToDxBrush(RenderTarget);
				SharpDX.Direct2D1.Brush brushDXask	= ColorAsk.ToDxBrush(RenderTarget);
				SharpDX.Direct2D1.Brush brushDXbid	= ColorBid.ToDxBrush(RenderTarget);
				
				lock (cdict_dictionaries)
				{
				if (cdict_dictionaries.Count > 0)
				{
					if (OutputOn)
						Print("OnRender: cdict_dictionaries has " + cdict_dictionaries.Count + " bar dictionaries.") ;

					for (int index = ChartBars.FromIndex; index <= ChartBars.ToIndex; index++) 
					{	//	 ChartBars.ToIndex == current bar

						// Process each dictionary for the visible bars
						double d_divisor =	0 		;// initialize max size to use as opacity divisor
						float f_barxpos	= chartControl.GetXByBarIndex(ChartBars, index) ;// X position of specific bar centerline
						DateTime dt_bartime = chartControl.GetTimeByX((int)f_barxpos) ;// time stamp of specific bar

						if (cdict_dictionaries.ContainsKey(index))
						{
							lock (cdict_dictionaries[index]) 
							{
							// Process each price in the visible bar's dictionary
							foreach (KeyValuePair<double, VolumeInfoItem> keyValue in cdict_dictionaries[index])
							{
								double d_price = keyValue.Key;
								VolumeInfoItem vii = keyValue.Value;
								
								double	d_barbottom	= d_price - (Bars.Instrument.MasterInstrument.TickSize / 2) ;// half tick lower than price
								float	f_ylower	= (float)chartScale.GetYByValueWpf(d_barbottom);
								float	f_yupper	= (float)chartScale.GetYByValueWpf(d_barbottom + Bars.Instrument.MasterInstrument.TickSize);// half tick higher than price
								if (f_yupper < 0 || f_ylower > chartScale.Height)// beyond chart top (<0) or bottom (>chartScale.Height)
									continue ;// skip to next keyValue.Key price
								
								// Get rectangle's left edge X position
								float f_xl = (float)(chartControl.GetXByBarIndex(ChartBars, index)) ;
								// Get rectangle's height
								float	f_height	= Math.Max(1, Math.Abs(f_yupper - f_ylower) - 1);
								
								// Fill the rectangles
								if (cui_maxliquidity > 0)
								{
									if (vii.volatask > cui_threshold)
									{	// Draw the sell side rectangles (red)
										brushDXask.Opacity = CalcOpacity(index, (float)vii.volatask) ;
										RenderTarget.FillRectangle(new SharpDX.RectangleF(f_xl, f_yupper, 
											chartControl.Properties.BarDistance, f_height), 
											brushDXask.Opacity >= HighlightThreshold ? brushDXhilite : brushDXask);
									}
									if (vii.volatbid > cui_threshold)
									{	// Draw the buy side rectangles (blue)
										brushDXbid.Opacity = CalcOpacity(index, (float)vii.volatbid) ;
										RenderTarget.FillRectangle(new SharpDX.RectangleF(f_xl, f_yupper, 
											chartControl.Properties.BarDistance, f_height), 
											brushDXbid.Opacity >= HighlightThreshold ? brushDXhilite : brushDXbid);
									}
								}
							}// end foreach keyValue in cdict_dictionaries[index]
							} // lock
						} // ContainsKey(index)
					} // end for index: visible chart bars
				} // cdict_dictionaries.Count > 0
				} // lock
				brushDXhilite.Dispose() ;
				brushDXask.Dispose();
				brushDXbid.Dispose();
			}
			catch (Exception ex)	
			{
				Log(Core.Globals.Now.ToString("MM/dd/yy HH:mm:ss") 
					+ " TROUBLE! " + CS_NAME + ".OnRender"
					+ "\t" + ex.Message
					+ "\r\n\tSource:\t\t" + ex.Source				
					+ "\r\n\tStackTrace:\t" + ex.StackTrace
					+ "\r\n\tTargetSite:\t" + ex.TargetSite
					+ "\r\n\tChartBars:\t" + cs_barsinfo, 
					LogLevel.Error	);
			}					
		}
tradesmart is offline  
Reply With Quote
Old 01-31-2017, 04:33 PM   #6
NinjaTrader_Jesse
NinjaTrader Customer Service
 
NinjaTrader_Jesse's Avatar
 
Join Date: Mar 2014
Location: Denver, CO
Posts: 5,065
Thanks: 30
Thanked 1,291 times in 1,209 posts
Default

Hello,

This could come from the foreach you are using, have you tried using a For loop instead of Foreach or creating a temporary list of the data?

From:
Code:
foreach (KeyValuePair<double, VolumeInfoItem> keyValue in cdict_dictionaries[index])
to

Code:
for(int i = 0; i < cdict_dictionaries[index].Length; i++)
{

}
or

Code:
foreach (KeyValuePair<double, VolumeInfoItem> keyValue in cdict_dictionaries[index].ToList())

A foreach loop would not be safe to use when modifying a collection that it is iterating over, a for loop on the other hand has a specific index supplied with each iteration and can change in the case of a collection change. using a seperated collection that is not directly a pointer to the actual collection can work as well which would be the .ToList() statement.

Could you try either of these approaches instead of using foreach directly on the collection and see if the problem is resolved?


I look forward of being of further assistance,
NinjaTrader_Jesse is online now  
Reply With Quote
The following user says thank you to NinjaTrader_Jesse for this post:
Old 02-01-2017, 09:57 AM   #7
tradesmart
Senior Member
 
Join Date: Nov 2010
Posts: 267
Thanks: 122
Thanked 108 times in 80 posts
Default

Thanks, Jesse. I shall try your suggestions and post the results.
tradesmart is offline  
Reply With Quote
Old 02-02-2017, 01:45 PM   #8
tradesmart
Senior Member
 
Join Date: Nov 2010
Posts: 267
Thanks: 122
Thanked 108 times in 80 posts
Default

Code:
for(int i = 0; i < cdict_dictionaries[index].Length; i++)
Using this for-loop technique, how do I access the KeyValuePair?
tradesmart is offline  
Reply With Quote
Old 02-03-2017, 12:29 PM   #9
NinjaTrader_Jesse
NinjaTrader Customer Service
 
NinjaTrader_Jesse's Avatar
 
Join Date: Mar 2014
Location: Denver, CO
Posts: 5,065
Thanks: 30
Thanked 1,291 times in 1,209 posts
Default

Hello tradesmart,

To loop over a dictionary I would suggest to review the following information:
http://stackoverflow.com/questions/1...ictionary-in-c

One example provided was the following where dictionary would equal your dictionary:

Code:
for (int index = 0; index < dictionary.Count; index++) {
  var item = dictionary.ElementAt(index);
  var itemKey = item.Key;
  var itemValue = item.Value;
}
Because I am unable to test the complete item you are questioning, you would likely need to reduce the code being used and try to narrow down what specific logic is causing the exception to happen. In my prior post I had only noted some possibilities, you would need to likely reduce the code and continue testing to locate the specific reason. I could suggest to compose a new sample script with a simplified OnRender and see if you get the same error, if not you could compare differences between the working and non working versions.


Please let me know if I may be of additional assistance.
NinjaTrader_Jesse is online now  
Reply With Quote
The following user says thank you to NinjaTrader_Jesse for this post:
Old 02-09-2017, 05:57 AM   #10
tradesmart
Senior Member
 
Join Date: Nov 2010
Posts: 267
Thanks: 122
Thanked 108 times in 80 posts
Default

I tested 5 different iteration techniques. Both #1 and #2 are error-free. #4 and #5 demonstrate odd behavior, such as "flashing" rectangles that had already been drawn.

The only problem remaining is resource usage. If the number of visible bars on the chart excedes a certain number, in my case 750 or so, other windows begin to freeze. Both MarketAnalyzer and SuperDOM become unresponsive. Charts appear to be functioning, but they are lagging several minutes behind, which becomes apparent at session end, when they continue to update long after the close.

If I keep the number of visible bars in the neighborhood of 350, then all windows remain responsive and the charts update in real time.

Code:
switch (ScanMode) 
{
    case 1:    // 1. ToList() * ok
        foreach (KeyValuePair<double, VolumeInfoItem> kvp in cdict_dictionaries[i_barkey].ToList())
        {
            DrawTheRectangles(...) ;
        }
        break;
    case 2:    // 2. for loop * ok
        for (int index = 0; index < cdict_dictionaries[i_barkey].Count; index++) 
        {
            var item = cdict_dictionaries[i_barkey].ElementAt(index);
            DrawTheRectangles(...) ;
        }
        break;                                    
    case 3:    // 3. might throw "Collection was modified" error
        foreach (KeyValuePair<double, VolumeInfoItem> kvp in cdict_dictionaries[i_barkey]) // original
        {
            DrawTheRectangles(...) ;
        }
        break;
    case 4: // 4. AsParallel().ForAll *** not good
        cdict_dictionaries[i_barkey].AsParallel().ForAll(kvp => 
            { 
                DrawTheRectangles(...) ;
            }
        );
        break;
    case 5:    // 5. Parallel.ForEach *** not good
        Parallel.ForEach(cdict_dictionaries[i_barkey], kvp => 
            { 
                DrawTheRectangles(...) ;
            }
        );
        break;
}
tradesmart is offline  
Reply With Quote
Old 02-10-2017, 12:06 PM   #11
NinjaTrader_Jesse
NinjaTrader Customer Service
 
NinjaTrader_Jesse's Avatar
 
Join Date: Mar 2014
Location: Denver, CO
Posts: 5,065
Thanks: 30
Thanked 1,291 times in 1,209 posts
Default

Hello tradesmart,

The reason that your examples 1 and 2 are working in contrast to the other examples is that in the first example you are creating a new List of the existing data .ToList(). Once you do this, you are no longer iterating and modifying the same collection because you are using two identical collections at this point. The second example uses a for loop which we had discussed earlier. In the case of the other examples, these all iterate over the collection using foreach which is the other item we had discussed earlier.

Regarding the performance, I see in the samples you are listing the method DrawTheRectangles, is this a method that uses Drawing Objects that are rectangles or are you actually rendering Rectangles using the RenderTarget? This would make a performance impact in the case you are drawing a huge number of Drawing objects versus just rendering the visual representation. Again I would need to see the specific syntax being used to know how to point a specific direction on this topic. The easiest solution is to find what specifically is causing the resource usage by commenting out code and testing to find the reason.

I look forward to being of further assistance.
NinjaTrader_Jesse is online now  
Reply With Quote
Old 02-10-2017, 12:30 PM   #12
tradesmart
Senior Member
 
Join Date: Nov 2010
Posts: 267
Thanks: 122
Thanked 108 times in 80 posts
Default

My DrawTheRectangles() method called by OnRender() uses RenderTarget.FillRectangle() to draw a couple dozen per bar.

I decided to port the indicator over to NT7 to compare performance and resource usage. The NT7 SuperDOM and MarketAnalyzer windows do not experience the sluggish and ultimately frozen behavior as I've seen in NT8.0.4.0. All NT7 windows remain responsive even with 980 chart bars and their rendered rectangles. CPU usage is generally less than NT8 and while it might occasionally touch 50% for an instant, it quickly falls back down. NT7 memory usage typically remains in the 500MB range, vs. NT8 hovering around 700MB and even 1GB sometimes.

So, it would appear that this particular indicator performs much better in NT7.
tradesmart is offline  
Reply With Quote
Old 02-10-2017, 01:06 PM   #13
NinjaTrader_Jesse
NinjaTrader Customer Service
 
NinjaTrader_Jesse's Avatar
 
Join Date: Mar 2014
Location: Denver, CO
Posts: 5,065
Thanks: 30
Thanked 1,291 times in 1,209 posts
Default

Hello tradesmart,

In this case, because you are using OnRender, you would need to look at the logic you are using overall to find out what is building up or causing the resources to be greater than NT7.

I can not see from what has been provided what that may be, but this could be determined if you debug the script and reduce the code to find specifically the scenario that causes this. I would suggest looking at how you are dealing with resources like brushes, are you creating new brushes every pass to OnRender or using a variable that was created in OnRenderTargetChanged? There are quite a few variables that would go into why resources are being consumed but really the only way to determine why would be to debug the logic.

NT7 and NT8 are similar but are very different in the way items are rendered and how resources can be used. Because of this difference, you may need to modify the existing logic more than you first deemed necessary to update the syntax to work better with NT8.

http://ninjatrader.com/support/helpG...onrendertarget
This page is a good resource in case you are creating new brushes over and over, instead, you can create brushes and then just reference them. This can be hugely helpful in reducing resources if you are not already using this type of logic. This page also has some general tips surrounding Brushes.

I look forward to being of further assistance.
NinjaTrader_Jesse is online now  
Reply With Quote
The following user says thank you to NinjaTrader_Jesse for this post:
Old 02-10-2017, 01:52 PM   #14
tradesmart
Senior Member
 
Join Date: Nov 2010
Posts: 267
Thanks: 122
Thanked 108 times in 80 posts
Default

Thanks for your help, Jesse.

Of the two error-free dictionary iteration techniques, which would you say is the most efficient: the for loop or foreach.ToList()?

I shall look into using OnRenderTargetChanged() as recommended. Currently, I am declaring, assigning, and disposing within OnRender() itself:

Code:
SharpDX.Direct2D1.Brush brushDXhilite  = ColorHighlight.ToDxBrush(RenderTarget); 
SharpDX.Direct2D1.Brush brushDXask   = ColorAsk.ToDxBrush(RenderTarget);
SharpDX.Direct2D1.Brush brushDXbid    = ColorBid.ToDxBrush(RenderTarget);
... then loop through visible ChartBars to draw rectangles ...
... then, after the loop:
Code:
brushDXhilite.Dispose() ;
brushDXask.Dispose();
brushDXbid.Dispose();
tradesmart is offline  
Reply With Quote
Old 02-10-2017, 03:57 PM   #15
NinjaTrader_Jesse
NinjaTrader Customer Service
 
NinjaTrader_Jesse's Avatar
 
Join Date: Mar 2014
Location: Denver, CO
Posts: 5,065
Thanks: 30
Thanked 1,291 times in 1,209 posts
Default

Hello tradesmart,

Yes creating the resources over and over like this could certainly lead to more than usual resource usage. Using OnRenderTargetChanged should help somewhat with creating static brushes that can be used over and over. This concept can play into other objects you may use, this method would be useful for creating items only Once.

Regarding the lists, I could not really say which is more efficient in this case, likely you could perform a test to see which specifically works better for you. you could also use google to search this general question as this is really more of a C# question specifically. "is there a performance impact when calling .tolist"

I located the following post which gives some good general details, basically yes this does cause a performance impact but it is very unlikely you would see this impact unless you were working with a very large list of objects.
http://stackoverflow.com/questions/1...calling-tolist

I look forward to being of further assistance.
NinjaTrader_Jesse is online now  
Reply With Quote
The following user says thank you to NinjaTrader_Jesse for this post:
Reply

Tags
collection, enumeration, error

Thread Tools
Display Modes

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

BB code is On
Smilies are On
[IMG] code is On
HTML code is Off

Forum Jump

Similar Threads
Thread Thread Starter Forum Replies Last Post
NT8b9 - new variation - collection was modified; enumeration may not execute sledge Version 8 Beta 1 03-21-2016 02:19 PM
Nt8-B5-Exception: collection was modified; enumeration operation may not execute sledge Version 8 Beta 2 02-25-2016 06:49 PM
Modified Stochastics agassi Indicator Development 6 09-25-2014 09:57 AM
Modified DonchianChannel? thepcmd NinjaScript File Sharing Discussion 1 09-27-2009 11:07 PM
MODIFIED MACDUpDown pedroprada@bellsouth.net Indicator Development 1 01-01-2009 02:38 PM


All times are GMT -6. The time now is 03:40 PM.

Futures, foreign currency and options trading contains substantial risk and is not for every investor. An investor could potentially lose all or more than the initial investment. Risk capital is money that can be lost without jeopardizing ones financial security or lifestyle. Only risk capital should be used for trading and only those with sufficient risk capital should consider trading. Past performance is not necessarily indicative of future results. View Full Risk Disclosure.

CFTC Rules 4.41 - Hypothetical or Simulated performance results have certain limitations, unlike an actual performance record, simulated results do not represent actual trading. Also, since the trades have not been executed, the results may have under-or-over compensated for the impact, if any, of certain market factors, such as lack of liquidity. Simulated trading programs in general are also subject to the fact that they are designed with the benefit of hindsight. No representation is being made that any account will or is likely to achieve profit or losses similar to those shown.

This website is hosted and operated by NinjaTrader, LLC (“NT”), a software development company which owns and supports all proprietary technology relating to and including the NinjaTrader trading platform. NT is an affiliated company to NinjaTrader Brokerage (“NTB”), which is a NFA registered introducing broker (NFA #0339976) providing brokerage services to traders of futures and foreign exchange products. This website is intended for educational and informational purposes only and should not be viewed as a solicitation or recommendation of any product, service or trading strategy. No offer or solicitation to buy or sell securities, securities derivative or futures products of any kind, or any type of trading or investment advice, recommendation or strategy, is made, given, or in any manner endorsed by any NT affiliate and the information made available on this Web site is not an offer or solicitation of any kind. Specific questions related to a brokerage account should be sent to your broker directly. The content and opinions expressed on this website are those of the authors and do not necessarily reflect the official policy or position of NT or any of its affiliates. 

Vendors along with their websites, products and services, collectively referred to as (“Vendor Content”), are independent persons or companies that are in no manner affiliated with NT or any if its affiliates. NT or any of its affiliates are not responsible for, do not approve, recommend or endorse any Vendor Content referenced on this website and it’s your sole responsibility to evaluate Vendor Content. Please be aware that any performance information provided by a vendor should be considered hypothetical and must contain the disclosures required by NFA Rule 2-29(c). If you are interested in learning more about, or investigating the quality of, any such Vendor Content you must contact the vendor, provider or seller of such Vendor Content. No person employed by, or associated with, NT or any of its affiliates is authorized to provide any information about any such Vendor Content.