Skip to content

Commit

Permalink
Tweaks plus partial fix to memory leak problem
Browse files Browse the repository at this point in the history 
When loading certain streets e.g. Chavila Checker, the programme
exceeded 256 MB, even when just one snap loaded and only about 7 quoins.
Removing the cache for the image when it was unloaded appeared to fix
the problem
(processing/processing#1391).
There is still some memory problem which I think is down to Processing
as a simple load street snap/unload street snap showed that after the
unload, the memory didn't return to the initial level before the first
load - some memory is not being released. Not sure I can do anything
about that other than set the memory bar for the programme a bit higher
e.g. to 300mb.

As it is useful, on level 1 error tracing, memory information is dumped
out when loading/unloading images - can then be put in a spreadsheet
(import, split at ,) and graphed so can see what is happening.

Also changed quoin output info to only give the class name - type is
irrelevant as included in class name information. And improved some
tracing for when start processing an item on a street.
  • Loading branch information
@jarkW
jarkW committed Jun 26, 2016
1 parent 2baafa7 commit 77ed960
Show file tree
Hide file tree
Showing 6 changed files with 64 additions and 48 deletions.
9 changes: 7 additions & 2 deletions ItemInfo.pde
View file
Expand Up @@ -92,7 +92,7 @@ class ItemInfo
}

public boolean initialiseItemInfo()
{
{
// Now open the relevant I* file - use the version which has been downloaded/copied to OrigJSONs
// If it is not there then report an error
String itemFileName = workingDir + File.separatorChar + "OrigJSONs" + File.separatorChar+ itemTSID + ".json";
Expand Down Expand Up @@ -182,7 +182,6 @@ class ItemInfo

// Initialise for the item to be searched for
//itemImageBeingUsed = 0;

if (!resetReadyForNewItemSearch())
{
return false;
Expand Down Expand Up @@ -1193,6 +1192,12 @@ class ItemInfo
return true;
}

public void clearFragFind()
{
fragFind = null;
System.gc();
}

public boolean resetAsMissingQuoin()
{
// This function is called if it is a quoin found to have the same x,y as another quoin
Expand Down
11 changes: 11 additions & 0 deletions Memory.pde
View file
Expand Up @@ -16,4 +16,15 @@ public class Memory {
//" Used Memory: " + (instance.totalMemory() - instance.freeMemory()) +
" Max Memory: " + instance.maxMemory() / mb + (instance.totalMemory() - instance.freeMemory()) / mb, 1);
}

public void printUsedMemory(String info)
{
int mb = 1024 * 1024;

// get Runtime instance
Runtime instance = Runtime.getRuntime();
// Insert a comma - then allows these output lines to be put into a spreadsheet so can graph memory usage
printToFile.printDebugLine(this, "\"" + info + "\"," + (instance.totalMemory() - instance.freeMemory()) / mb, 1);

}
}
16 changes: 14 additions & 2 deletions PNGFile.pde
View file
Expand Up @@ -89,12 +89,14 @@ class PNGFile

public boolean loadPNGImage()
{
memory.printUsedMemory("image load start " + PNGImageName);
// Load up this snap/item image
String fullFileName;

if (PNGImage != null)
{
// Image has already been loaded into memory
memory.printUsedMemory("image load end (already loaded) " + PNGImageName);
return true;
}

Expand Down Expand Up @@ -128,6 +130,7 @@ class PNGFile
try
{
// load image pixels
memory.printUsedMemory("image load pre-loadPixels " + PNGImageName);
PNGImage.loadPixels();
}
catch(Exception e)
Expand All @@ -138,16 +141,25 @@ class PNGFile
}

printToFile.printDebugLine(this, "Loading image from " + fullFileName + " with width " + PNGImage.height + " height " + PNGImage.width, 1);

memory.printUsedMemory("image load end " + PNGImageName);
return true;
}

public void unloadPNGImage()
{
{
memory.printUsedMemory("image unload start " + PNGImageName);
if (PNGImage == null)
{
printToFile.printDebugLine(this, "!!!! Unloading null image " + PNGImageName, 3);
}
// workaround to remove memory leak - https://github.com/processing/processing/issues/1391.html#issuecomment-13356835
g.removeCache(PNGImage);

PNGImage = null;
printToFile.printDebugLine(this, "Unloading image " + PNGImageName, 1);
// Need this to force the garbage collection to free up the memory associated with the image
System.gc();
memory.printUsedMemory("image unload end " + PNGImageName);
}

}
30 changes: 10 additions & 20 deletions PrintToFile.pde
View file
Expand Up @@ -361,7 +361,7 @@ class PrintToFile {
if (itemResults.get(i).itemInfo.readItemClassTSID().equals("quoin"))
{
s = s + "MISSING quoin " + itemResults.get(i).itemInfo.readItemTSID() + ": " + itemResults.get(i).itemInfo.readItemClassTSID();
s = s + "(" + itemResults.get(i).itemInfo.readOrigItemVariant() + "/" + itemResults.get(i).itemInfo.readOrigItemClassName() + ")";
s = s + "(" + itemResults.get(i).itemInfo.readOrigItemClassName() + ")";
s = s + " defaulted to (mystery/placement tester)";
}
else
Expand All @@ -376,7 +376,7 @@ class PrintToFile {
s = s + "Changed co-ords " + itemResults.get(i).itemInfo.readItemTSID() + ": " + itemResults.get(i).itemInfo.readItemClassTSID();
if (itemResults.get(i).itemInfo.readItemClassTSID().equals("quoin"))
{
s = s + "(" + itemResults.get(i).itemInfo.readOrigItemVariant() + "/" + itemResults.get(i).itemInfo.readOrigItemClassName() + ")";
s = s + "(" + itemResults.get(i).itemInfo.readOrigItemClassName() + ")";
}
else
{
Expand All @@ -388,8 +388,8 @@ class PrintToFile {
s = s + "Changed variant " + itemResults.get(i).itemInfo.readItemTSID() + ": " + itemResults.get(i).itemInfo.readItemClassTSID();
if (itemResults.get(i).itemInfo.readItemClassTSID().equals("quoin"))
{
s = s + "(" + itemResults.get(i).itemInfo.readNewItemVariant() + "/" + itemResults.get(i).itemInfo.readNewItemClassName() + ")";
s = s + " (was " + itemResults.get(i).itemInfo.readOrigItemVariant() + "/" + itemResults.get(i).itemInfo.readOrigItemClassName() + ")";
s = s + "(" + itemResults.get(i).itemInfo.readNewItemClassName() + ")";
s = s + " (was " + itemResults.get(i).itemInfo.readOrigItemClassName() + ")";
}
else
{
Expand All @@ -409,8 +409,8 @@ class PrintToFile {
s = s + "Changed variant & co-ords " + itemResults.get(i).itemInfo.readItemTSID() + ": " + itemResults.get(i).itemInfo.readItemClassTSID();
if (itemResults.get(i).itemInfo.readItemClassTSID().equals("quoin"))
{
s = s + "(" + itemResults.get(i).itemInfo.readNewItemVariant() + "/" + itemResults.get(i).itemInfo.readNewItemClassName() + ")";
s = s + " (was " + itemResults.get(i).itemInfo.readOrigItemVariant() + "/" + itemResults.get(i).itemInfo.readOrigItemClassName() + ")";
s = s + "(" + itemResults.get(i).itemInfo.readNewItemClassName() + ")";
s = s + " (was " + itemResults.get(i).itemInfo.readOrigItemClassName() + ")";
}
else
{
Expand All @@ -427,22 +427,12 @@ class PrintToFile {
break;

case SummaryChanges.UNCHANGED:
/*
if (itemResults.get(i).readAlreadySetDirField())
{
// This handles the case of e.g. shrine where x,y unchanged and just inserted the missing dir field
// Had to fake up the missing variant field in order to correctly handle images
s = s + "Changed variant ";
s = s + itemResults.get(i).itemInfo.readItemTSID() + ": " + itemResults.get(i).itemInfo.readItemClassTSID();
}
else
{*/
s = s + "Unchanged ";
s = s + itemResults.get(i).itemInfo.readItemTSID() + ": " + itemResults.get(i).itemInfo.readItemClassTSID();
//}
s = s + "Unchanged ";
s = s + itemResults.get(i).itemInfo.readItemTSID() + ": " + itemResults.get(i).itemInfo.readItemClassTSID();

if (itemResults.get(i).itemInfo.readItemClassTSID().equals("quoin"))
{
s = s + "(" + itemResults.get(i).itemInfo.readOrigItemVariant() + "/" + itemResults.get(i).itemInfo.readOrigItemClassName() + ")";
s = s + "(" + itemResults.get(i).itemInfo.readOrigItemClassName() + ")";
}
else
{
Expand Down
42 changes: 19 additions & 23 deletions StreetInfo.pde
View file
Expand Up @@ -763,8 +763,13 @@ class StreetInfo
{
// Item needs to be skipped/or has already been found
// Move onto next one
printToFile.printDebugLine(this, "Skipping item " + itemInfo.get(itemBeingProcessed).readItemClassTSID() + "(" +
itemInfo.get(itemBeingProcessed).readOrigItemVariant() + ") " + itemInfo.get(itemBeingProcessed).readItemTSID(), 1);
String s = "Skipping item " + itemInfo.get(itemBeingProcessed).readItemClassTSID();
if (itemInfo.get(itemBeingProcessed).readOrigItemVariant().length() > 0)
{
s = s + " (" + itemInfo.get(itemBeingProcessed).readOrigItemVariant() + ")";
}
s = s + " " + itemInfo.get(itemBeingProcessed).readItemTSID();
printToFile.printDebugLine(this, s, 1);

// As we just want to pass control back up, don't care about the succes/failure - top level will handle that
if (moveToNextItem())
Expand All @@ -780,7 +785,6 @@ class StreetInfo
//ItemInfo itemData = itemInfo.get(itemBeingProcessed);

// Display information
//displayMgr.setStreetName(streetName, streetTSID, streetBeingProcessed + 1, configInfo.readTotalJSONStreetCount());
displayMgr.setItemProgress(itemInfo.get(itemBeingProcessed).itemClassTSID, itemInfo.get(itemBeingProcessed).itemTSID, itemBeingProcessed+1, itemInfo.size());

// Search the snap for this image/item
Expand All @@ -797,32 +801,21 @@ class StreetInfo
if (!moveToNextItem())
{
// Either error condition or at end of street/items - so need to return to top level to start over with new snap/street
//failNow = true;
return;
}
else
{
// Next item is safe to procced to

// Set up fragFind in item ready to start the next item/streetsnap search combo
// i.e. loads up pointers to correct street snap and item images
// Only do this for items we still need to search for
if (itemValidToContinueSearchingFor(itemBeingProcessed))
{
if (!itemInfo.get(itemBeingProcessed).resetReadyForNewItemSearch())
{
displayMgr.showErrMsg("Unexpected error getting ready for new item search", true);
failNow = true;
return;
}
printToFile.printDebugLine(this, "PROCESSING ITEM " + itemBeingProcessed + " ON STREET SNAP " + streetSnapBeingUsed, 1);
printToFile.printDebugLine(this, "PROCESSING ITEM " + itemBeingProcessed + "(" + itemInfo.get(itemBeingProcessed).readItemTSID() + ") ON STREET SNAP " + streetSnapBeingUsed, 1);
}
else
{
printToFile.printDebugLine(this, "Skipping item/item Found " + itemInfo.get(itemBeingProcessed).readItemClassTSID() + "(" +
itemInfo.get(itemBeingProcessed).readOrigItemVariant() + ") " +
itemInfo.get(itemBeingProcessed).readItemTSID(), 1);
}
}
}

}
Expand All @@ -841,6 +834,11 @@ class StreetInfo
{
// Finished all items on the street
// So move onto the next street snap after unloading the current one

// But first need to null the FragFind structure in the structure for the last item - as it contains a reference to the street snap
// which means the call below to unload the street snap won't work - and then memory hell ensues.
itemInfo.get(itemBeingProcessed-1).clearFragFind();

streetSnaps.get(streetSnapBeingUsed).unloadPNGImage();

// reset itemBeingProcessed back to 0
Expand Down Expand Up @@ -879,7 +877,7 @@ class StreetInfo
// Now print out the summary array
// The second sorting of item results shouldn't throw up any duplicate x,y - if it happens they'll just be reported as warnings.
// Any actual errors are reported from within printSummaryData
if (! printToFile.printSummaryData(itemResults))
if (!printToFile.printSummaryData(itemResults))
{
failNow = true;
return false;
Expand All @@ -902,8 +900,7 @@ class StreetInfo
return false;
}
itemBeingProcessed = 0;
printToFile.printDebugLine(this, "STARTING WITH FIRST ITEM ON STREET SNAP " + streetSnapBeingUsed, 1);

printToFile.printDebugLine(this, "STARTING WITH FIRST ITEM (" + itemInfo.get(itemBeingProcessed).readItemTSID() + ") ON STREET SNAP " + streetSnapBeingUsed, 1);
if (itemValidToContinueSearchingFor(itemBeingProcessed))
{
if (!itemInfo.get(itemBeingProcessed).resetReadyForNewItemSearch())
Expand All @@ -912,11 +909,10 @@ class StreetInfo
failNow = true;
return false;
}
printToFile.printDebugLine(this, "PROCESSING ITEM " + itemBeingProcessed + " ON STREET SNAP " + streetSnapBeingUsed, 1);
printToFile.printDebugLine(this, "PROCESSING ITEM " + itemBeingProcessed + " (" + itemInfo.get(itemBeingProcessed).readItemTSID() + ") ON STREET SNAP " + streetSnapBeingUsed, 1);
}

}
//return false;

} // if past end of item list
else
Expand All @@ -930,7 +926,7 @@ class StreetInfo
failNow = true;
return false;
}
printToFile.printDebugLine(this, "PROCESSING ITEM " + itemBeingProcessed + " ON STREET SNAP " + streetSnapBeingUsed, 1);
printToFile.printDebugLine(this, "PROCESSING ITEM " + itemBeingProcessed + " (" + itemInfo.get(itemBeingProcessed).readItemTSID() + ") ON STREET SNAP " + streetSnapBeingUsed, 1);
}
}
return true;
Expand Down Expand Up @@ -1257,7 +1253,7 @@ class StreetInfo
}
return true;
}

void initStreetItemVars()
{
for (int i = 0; i < itemInfo.size(); i++)
Expand Down
4 changes: 3 additions & 1 deletion sketch_QA_tool.pde
View file
Expand Up @@ -196,6 +196,7 @@ public void draw()
// Carry out processing depending on whether setting up the street or processing it
//println("nextAction is ", nextAction);
//memory.printMemoryUsage();
//memory.printUsedMemory("start");
switch (nextAction)
{
case IDLING:
Expand Down Expand Up @@ -383,7 +384,7 @@ public void draw()
// Carries out the setting up of the street and associated items
printToFile.printDebugLine(this, "Timestamp: " + nf(hour(),2) + nf(minute(),2) + nf(second(),2), 1);
printToFile.printDebugLine(this, "Read street data for TSID " + configInfo.readStreetTSID(streetBeingProcessed), 2);

if (!initialiseStreet())
{
// fatal error
Expand Down Expand Up @@ -581,6 +582,7 @@ public void draw()
printToFile.printDebugLine(this, "Unexpected next action - " + nextAction, 3);
exit();
}
//memory.printUsedMemory("end");
}

void doExitCleanUp()
Expand Down

0 comments on commit 77ed960

Please sign in to comment.