Collection was modified

[tradesmart]

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)

cdict_barvolume = new Dictionary<double, VolumeInfoItem>();
if (!cdict_barvolumedictionaries.ContainsKey(CurrentBar))
   cdict_barvolumedictionaries.Add(CurrentBar, cdict_barvolume);

[B]lock [/B](e.Instrument.SyncMarketData) //[B] will this be helpful?[/B]
{
   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)

[B]lock [/B](cdict_barvolumedictionaries) // [B]will this be helpful?[/B]
{
   for (int index = ChartBars.FromIndex; index <= ChartBars.ToIndex; index++) 
    {
      [B]lock [/B](cdict_barvolumedictionaries[index]) // [B]will this be helpful?[/B]
      {
         // ... draw some objects using the dictionary's data
      }
    }
}

[NinjaTrader_Jesse]

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.

[tradesmart]

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.

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

[NinjaTrader_Jesse]

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.

[tradesmart]

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.

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.

		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	);
			}					
		}

[NinjaTrader_Jesse]

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:

foreach (KeyValuePair<double, VolumeInfoItem> keyValue in cdict_dictionaries[index])

to

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

}

or

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

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,

[tradesmart]

Thanks, Jesse. I shall try your suggestions and post the results.

[tradesmart]

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

Using this for-loop technique, how do I access the KeyValuePair?

[NinjaTrader_Jesse]

Hello tradesmart,

To loop over a dictionary I would suggest to review the following information:

http://stackoverflow.com/questions/141088/what-is-the-best-way-to-iterate-over-a-dictionary-in-c

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

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.

[tradesmart]

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.

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;
}

[NinjaTrader_Jesse]

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.

[tradesmart]

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.

[NinjaTrader_Jesse]

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/helpGuides/nt8/en-us/onrendertargetchanged.htm?zoom_highlightsub=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.

[tradesmart]

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:

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:

brushDXhilite.Dispose() ;
brushDXask.Dispose();
brushDXbid.Dispose();

[NinjaTrader_Jesse]

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/15516462/is-there-a-performance-impact-when-calling-tolist

I look forward to being of further assistance.