Bugzilla – Full Text Bug Listing
| Summary: | Proposal for startup performance improvement | ||
|---|---|---|---|
| Product: | [RT] Virgo | Reporter: | Nikolai Tankov <nikolai.tankov> |
| Component: | unknown | Assignee: | Steve Powell <zteve.powell> |
| Status: | CLOSED FIXED | QA Contact: | |
| Severity: | enhancement | ||
| Priority: | P3 | CC: | glyn.normington, zteve.powell |
| Version: | unspecified | ||
| Target Milestone: | --- | ||
| Hardware: | PC | ||
| OS: | Windows XP | ||
| Whiteboard: | |||
(In reply to comment #0) Thanks Nikolai, This is a really useful patch suggestion. I notice that the code applies quite well to single-level directories, and for the un/stashContent() methods explicitly. I would like to make the 'move' function available more generally, and so I want to generalise your patch to a method called 'moveTo' on org.eclipse.virgo.util.io.PathReference. In order to understand it better I have a few questions: - can you explain why you don't rename directories directly? does this fail on Windows? It should work fine on Unix. - when rename of a directory is required the immediate children are renamed first (but the top-level directory is not renamed) and the source directory is then deleted (it is presumably empty). What happens if the children are directories? I also want to make a few suggestions of my own: - it would be better if the result of the File.renameTo() methods was not ignored. If any of the renames fail (this might be a hard failure if the rename crosses filesystems) the resulting mess could be considerable. - if there are pre-conditions on the use of the method StandardArtifactStorage#move(File, File) they should be documented in the comments of the method. This patch suggestion is really important to us and this will turn out to be a very valuable contribution to the code base. I have a proposed change to PathReference (and to the un/stashContent() methods to call it) which I will post here when it is finalised. Steve Powell The git commits on util:
Fri May 21 2010 17:43:00 60ecdfbc05 (Added moveTo() operation to PathReference (plus tests).)
Wed Jun 09 2010 16:41:44 d92ba88c69 (Added explicit test for moveTo to check that fast move works.)
Wed Jun 09 2010 20:19:54 530c29e26a (Add test for coverage of moveTo() method on PathReference.)
added the moveTo() method on class PathReference; which is used extensively in kernel code for file manipulation.
The git commit on kernel:
Tue Jun 15 2010 12:46:30 8dda7f9a06 (Exploit PathReference.moveTo() in kernel deployer for performance.)
modified StandardArtifactStorage and DeployerRecoveryLog to use moveTo().
These changes made it into 2.1.0.M01.
The performance improvements were not re-measured, and although expected to be modest, combined with other improvements to startup we should see benefit from this change.
Done |
Hi , Startup is too slow on Windows and I experimented with following changes in ...kernel.install.artifact.internal.StandardArtifactStorage The changes are in 2 methods : private void stashContent() { if (this.baseStagingPathReference.exists()) { this.pastStagingPathReference.delete(true); //move from base to past recursively //this.baseStagingPathReference.copy(this.pastStagingPathReference, true); //this.baseStagingPathReference.delete(true); move(pastStagingPathReference.toFile(), baseStagingPathReference.toFile()); } } private void unstashContent() { if (this.pastStagingPathReference.exists()) { this.baseStagingPathReference.delete(true); //move from past to base recursively //this.pastStagingPathReference.copy(this.baseStagingPathReference, true); //this.pastStagingPathReference.delete(true); move(baseStagingPathReference.toFile(), pastStagingPathReference.toFile()); } } And one additional method was added : public static void move(File target, File source) { if (source.exists()) { if (source.isFile()) { source.renameTo(target); } else { String[] files = source.list(); if (files != null) { for (String name : files) { File sourceFile = new File(source, name); File targetFile = new File(target, name); sourceFile.renameTo(targetFile); } } source.delete(); } } } With this simple change I was able to achieve 5-8% improvement on my machine. Probably same optimization is also applicable in other classes but I did not checked. Best regards, Nikolai.