Jump to content


Better granularity needed for progress bar


8 replies to this topic

#1 Orions_Stardom

  • Gibberlings
  • 206 posts

Posted 11 February 2007 - 08:02 PM

Currently, recursion for copying directories is done in FileSystem.copy(), and the clone logic simply tells it to copy 'data'. This leads to the progressbar to simply say it's copying 'data', which can take a while. I need to update this, so that the clone logic generates a full recursive list of everything that will be copied, and updates the progress bar appropriately.

#2 Orions_Stardom

  • Gibberlings
  • 206 posts

Posted 12 February 2007 - 03:20 AM

I introduced the following code into FileSystem.java, at line 170 (after 'public static boolean canLink()'):
public static String[] listAllFiles(File dir){
	LinkedList<String> ret = new LinkedList<String>();
	for(File f : dir.listFiles()){
		if(f.isDirectory()){
			for(String s : listAllFiles(f)){
				ret.add(dir.getName() + File.SEPARATOR + s);
			}
		}
		else{
			ret.add(f.getName());
		}
	}
	return ret.toArray(new String[0]);
}

And the following into CloneWorker.java, at line 128 (after the checks to add music, data, et al to requiredFiles):
// Recurse into any directories we're copying
for(String s : requiredFiles){
	File f = new File(source, s);
	if(!f.isDirectory()){
		continue;
	}
	requiredFiles.remove(s);
	requiredFiles.addAll(
		Arrays.asList(FileSystem.listAllFiles(f)));
}

What this is meant to do is fix this bug. What it *actually* does is to break things before being called - in particular, the top code stops checkboxes being enabled when a source is selected, and stops the progress bar from updating while the clone is happening - I think it may also be stopping the clone from happening at all.

But note that until the clone is actually happening, there is no path to either of these sections. There is certainly no way that either can happen when a source is selected to stop the checkboxes becoming enabled.

But diff(1) tells me that this is all I changed, and commenting out both of these code sections makes everything work as well as it did before.

I am confused.

#3 Orions_Stardom

  • Gibberlings
  • 206 posts

Posted 12 February 2007 - 04:14 AM

God I'm an idiot. This was a problem with a broken build script, which gave me a .jar, and deleted the error log, even when there was actually an error in the form of File.SEPARATOR not existing, because the standard libraries don't capitalise names of constants. There'll be a build up with this fix once I fix some threading bugs that just popped up...

#4 Orions_Stardom

  • Gibberlings
  • 206 posts

Posted 12 February 2007 - 04:57 AM

Ok, to get this working I also had to change the directory-swapping logic to:
for(ListIterator it = requiredFiles.listIterator(); it.hasNext(); ){
	File f = new File(source, it.next().toString());
	if(!f.isDirectory()){
		continue;
	}
	it.remove();
	for(String s : FileSystem.listAllFiles(f)){
		it.add(s);
	}
}
Turns out using add(), addAll(), or remove() on a list you're iterating over (even with the enhanced-for loop) triggers a ConcurrentModificationException - to avoid this, it needs to be done through the ListIterator's own remove() and add() methods.

#5 Orions_Stardom

  • Gibberlings
  • 206 posts

Posted 12 February 2007 - 05:01 AM

One of the fixes I put in 5.1.3 has completely stopped the clone process from ever actually *doing* anything. It pretends to, though, and can be cancelled. My gut instinct is that it will be the fake-data-files stuff, but I'll check this after I get some sleep.

#6 Orions_Stardom

  • Gibberlings
  • 206 posts

Posted 12 February 2007 - 12:57 PM

Ok, my gut was wrong. This is caused by the progress bar granulation code. I'm going to put up a build with that commented out, and merge that thread into here.

#7 Orions_Stardom

  • Gibberlings
  • 206 posts

Posted 12 February 2007 - 08:38 PM

There appear to be two separate problems here. The first is that listing all files in all directories required takes ages, and nothing can happen until it is complete.

The second is that it doesn't do what it's meant to. Waiting long enough, the tool gives 'FileNotFoundException: script compiler\ACTION.IDS' - which is the first file in the first directory to be copied.

#8 Orions_Stardom

  • Gibberlings
  • 206 posts

Posted 12 February 2007 - 09:13 PM

First attempt to fix the slowness - just count the files first, and use a callback from copy() to update the progress bar. The idea being that the slowness in listAllFiles() could be related to the fact that it - for every sub directory - concatanates two lists (an O(listsize) operation) and converts between an array and a list (another O(listsize)) twice. But the following code to simply count the files takes just as long:
public static int countFiles(File dir){
	if(!dir.isDirectory()){
		return 1;
	}
	int ret = 0;
	for(File f : dir.listFiles()){
		ret += countFiles(f);
	}
	return ret;
}


#9 Orions_Stardom

  • Gibberlings
  • 206 posts

Posted 12 February 2007 - 11:31 PM

I tried getting this done in a background thread, but it doesn't work all that well - the copying overtakes the calculating very quickly, and synchronising them at various points would still have the apparent-slowness problem.
I think the only way to do this would be to hard-code the full contents of directories into CloningData.java - but that can't be done with music, override, portraits, save, and possibly script compiler and characters; it's also possibly mod-unfriendly for data . Moving to No Action.




Reply to this topic



  


1 user(s) are reading this topic

0 members, 1 guests, 0 anonymous users