Re: [patch] NioFileUtils, Java6FileUtils, FileUtilsAdapter + factory, build.xml (was Re: AW: Adding a methof to StringUtils)
Kev Jackson wrote: > > I'm coming round to think that Martijn's approach would be the least > disruptive to the code base - simply as the delegate approach wouldn't > mean recoding all tasks to use an interface instead of an > implementation. This reduces the effort a lot and would make it less > likely that errors would creep in. Definitely my +1 for the delegate > approach, +0 for the interfaces and factory approach (ie if for some > reason we can't do the delagate approach, the interfaces should be > reconsidered) > Sounds good. When you plugin the nio, it would be good if you test the following : - building under 1.4 or 1.5, running under 1.2 - I am also concerned about users building their own ant distribution under Java 1.2, then running it under 1.4 and 1.5, and then complaining that org.apache.tools.ant.types.jdk14 cannot be found. Similarly, I wonder what happens for a user who would 1) build ant under JDK 1.2, 2) run ant under JDK 1.4, and attempt to use the RegexpFactory, for instance through the task ?. The RegexpFactory does not check whether org.apache.tools.ant.util.regexp.Jdk14RegexpRegexp is present, only whether java.util.regex.Matcher is present. Actually, I was wondering whether we are not already forbidding - de facto and silently - this type of use cases. Regards, Antoine Antoine > Thanks > Kev > - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: [patch] NioFileUtils, Java6FileUtils, FileUtilsAdapter + factory, build.xml (was Re: AW: Adding a methof to StringUtils)
FileUtils to select (Martijn's approach is ridiculously insane enough to have captured my fancy ;) ), it must also take into account whether the desired classes are available, falling back to the basic impl. otherwise. I'm coming round to think that Martijn's approach would be the least disruptive to the code base - simply as the delegate approach wouldn't mean recoding all tasks to use an interface instead of an implementation. This reduces the effort a lot and would make it less likely that errors would creep in. Definitely my +1 for the delegate approach, +0 for the interfaces and factory approach (ie if for some reason we can't do the delagate approach, the interfaces should be reconsidered) Thanks Kev - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: [patch] NioFileUtils, Java6FileUtils, FileUtilsAdapter + factory, build.xml (was Re: AW: Adding a methof to StringUtils)
--- Antoine Levy-Lambert <[EMAIL PROTECTED]> wrote: [SNIP] > > If anyway ResourceUtils does the work now, > ResourceUtils could test > whether running under Java 1.4 or more and call > NIOUtils to do the work > happening between lines 364 to 379 of ResourceUtils > (copying files with no filters, no bells and > whistles). > Shouldn't we simply extend nio copy support to ResourceUtils? Another version-dependent delegate could use Channels.newChannel() with the resources' Input/OutputStreams... assuming that the implementation is smart enough to call getChannel() against a FileInputStream, for example. We could test this; if it doesn't work we can always code that ourselves. Could be overall worthwhile if we would pick up performance increases on e.g. url streams... -Matt > Regards, > > Antoine > > > - > To unsubscribe, e-mail: > [EMAIL PROTECTED] > For additional commands, e-mail: > [EMAIL PROTECTED] > > __ Do You Yahoo!? Tired of spam? Yahoo! Mail has the best spam protection around http://mail.yahoo.com - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: [patch] NioFileUtils, Java6FileUtils, FileUtilsAdapter + factory, build.xml (was Re: AW: Adding a methof to StringUtils)
--- Antoine Levy-Lambert <[EMAIL PROTECTED]> wrote: [SNIP] > One issue : > > If NIOFileUtils is in the same package as FileUtils, > bootstrapping under > JDK 1.2 or 1.3 would not work. > The bootstrap scripts (build.bat, build.sh) are > using primitive > mechanisms (directories) to find out what should be > built first. > Oh yeah... forgot about this! :) Now I see what you meant here. We must keep that in mind... the bootstrap scripts build enough of Ant that it can build the rest of itself, itself... correct? Meaning that whatever mechanism is used to determine which FileUtils to select (Martijn's approach is ridiculously insane enough to have captured my fancy ;) ), it must also take into account whether the desired classes are available, falling back to the basic impl. otherwise. Right? -Matt __ Do You Yahoo!? Tired of spam? Yahoo! Mail has the best spam protection around http://mail.yahoo.com - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: [patch] NioFileUtils, Java6FileUtils, FileUtilsAdapter + factory, build.xml (was Re: AW: Adding a methof to StringUtils)
Kev Jackson wrote: > >> In that case I would like some time to think things over, as to >> provide one face, and to have different implementations for currently >> different parts. >> Classes that support copying, things that support permissions, things >> that support path manipulation, without getting a really big number >> of classes. >> >> FileUtils for Java 1.6 on Windows >> FileUtils for Java 1.4 on VMS >> etc. >> >> I had an idea to turn FileUtils into a Facade that delegates >> different actions to different back-ends. >> > I think I can see where you are going with this, and it's definitely > an alternative to what I thought to begin with and the direction I was > heading in the code. I'm viewing the cleanest split as using an > interface, from your perspective you think a master FileUtils (class > not interface) that delegates all the real work to a specialization > class. I think that your view would provide the quickest benefit - ie > it would be more easily added to the entire codebase (as there would > be no need to remove references to FileUtils from all the tasks). > > Thanks for your thoughts > Kev > Hi, in this case, what we could do would be to create a new package org.apache.tools.ant.util.java14 (this is OK for bootstrapping and build.xml) and create there a class NIOUtils or something like that with some static methods, including simply one method which copies one file to the other. If anyway ResourceUtils does the work now, ResourceUtils could test whether running under Java 1.4 or more and call NIOUtils to do the work happening between lines 364 to 379 of ResourceUtils (copying files with no filters, no bells and whistles). Regards, Antoine - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: [patch] NioFileUtils, Java6FileUtils, FileUtilsAdapter + factory, build.xml (was Re: AW: Adding a methof to StringUtils)
Hello Kevin, Kev Jackson wrote: > Ok, this is what I've managed to bash together this morning: > > - NioFileUtils (uses Java1.4 NIO libraries, altered the TRANSFER_SIZE > as suggested to 32000, currently no way to override that (yet)) > - Java6FileUtils (currently has no implementation, but could be used > in future for implementing Java6 features (file permisions etc)) > - FileUtils (refactored to imlement interface, still needs to have all > comments refactored into @see -> interface) > - FileUtilsAdapter (interface of FileUtils) > - FileUtilsFactory (uses runtime class-loading to select correct > version, also allows for selection of impl via system property or ant > property (project.getProperty), as suggested, caches the instance of > the FileUtilsAdapter on first use. This assumes that during a build a > user won't want to swap out FilUtils implementations.) > - build.xml (contains the selector code for [Nio|Java6]FileUtils) > > Current problems: > I cannot get the current code to select the implementation based on > the property I'm specifying - (I'm using .ant.properties and I've also > tried -D style props). Any hint as to how this mechanism works in > general would be appreciated. Did you try to debug it ? It seems to work well for regular expressions. > Code is just in util package, it would be nicer in it's own separate > package - open to suggestions on this. > Not sure that the selector in the build is working - after bootstrap + > build, the ant-nodeps.jar doesn't contain the Nio or the > Java6FileUtils - should they be here? > > Patch attached for consideration and comments. > One issue : If NIOFileUtils is in the same package as FileUtils, bootstrapping under JDK 1.2 or 1.3 would not work. The bootstrap scripts (build.bat, build.sh) are using primitive mechanisms (directories) to find out what should be built first. > Btw, swapping to NIO does help the AppFuse build to get past the > copying - it now fails on HtmlUnit - which we can't do much about. So > (anecdotally anyway), NIO seems to use less memory than traditional IO. > Good news. > Thanks > Kev > Regards, Antoine - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: [patch] NioFileUtils, Java6FileUtils, FileUtilsAdapter + factory, build.xml (was Re: AW: Adding a methof to StringUtils)
Ist es nicht schrecklich, dass der menschlichen Klugheit so enge Grenzen gesetzt sind und der menschlichen Dummheit überhaupt keine? (Konrad Adenauer) Isn't it terrible that so close borders are set to the human intelligence and the human stupidity at all none? This is a nice quote (totally OT I hope :) It never is totally off topic, and is definitely including myself. The sheer fact that it came through is my own stupidity. (For some reason one of my mail clients puts this signature on top of every reply, which I normally kill out when sending things to mailing lists). Martijn - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: [patch] NioFileUtils, Java6FileUtils, FileUtilsAdapter + factory, build.xml (was Re: AW: Adding a methof to StringUtils)
Ist es nicht schrecklich, dass der menschlichen Klugheit so enge Grenzen gesetzt sind und der menschlichen Dummheit überhaupt keine? (Konrad Adenauer) Isn't it terrible that so close borders are set to the human intelligence and the human stupidity at all none? This is a nice quote (totally OT I hope :) On Fri, 21 Apr 2006, Kev Jackson wrote: I'm viewing the cleanest split as using an interface, from your perspective you think a master FileUtils (class not interface) that delegates all the real work to a specialization class. Not exactly, I am thinking about delegating to different specialization classes. Some aspects depend on java version, some aspects on os version, some aspects on availability of 3pp. To be able to handle that I think it is better to split functionality up. sorry yes in my mail I said class when of course I meant class(es) plural. Kev - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: [patch] NioFileUtils, Java6FileUtils, FileUtilsAdapter + factory, build.xml (was Re: AW: Adding a methof to StringUtils)
Ist es nicht schrecklich, dass der menschlichen Klugheit so enge Grenzen gesetzt sind und der menschlichen Dummheit überhaupt keine? (Konrad Adenauer) Isn't it terrible that so close borders are set to the human intelligence and the human stupidity at all none? On Fri, 21 Apr 2006, Kev Jackson wrote: I'm viewing the cleanest split as using an interface, from your perspective you think a master FileUtils (class not interface) that delegates all the real work to a specialization class. Not exactly, I am thinking about delegating to different specialization classes. Some aspects depend on java version, some aspects on os version, some aspects on availability of 3pp. To be able to handle that I think it is better to split functionality up. Martijn - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: [patch] NioFileUtils, Java6FileUtils, FileUtilsAdapter + factory, build.xml (was Re: AW: Adding a methof to StringUtils)
In that case I would like some time to think things over, as to provide one face, and to have different implementations for currently different parts. Classes that support copying, things that support permissions, things that support path manipulation, without getting a really big number of classes. FileUtils for Java 1.6 on Windows FileUtils for Java 1.4 on VMS etc. I had an idea to turn FileUtils into a Facade that delegates different actions to different back-ends. I think I can see where you are going with this, and it's definitely an alternative to what I thought to begin with and the direction I was heading in the code. I'm viewing the cleanest split as using an interface, from your perspective you think a master FileUtils (class not interface) that delegates all the real work to a specialization class. I think that your view would provide the quickest benefit - ie it would be more easily added to the entire codebase (as there would be no need to remove references to FileUtils from all the tasks). Thanks for your thoughts Kev - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: [patch] NioFileUtils, Java6FileUtils, FileUtilsAdapter + factory, build.xml (was Re: AW: Adding a methof to StringUtils)
Kev Jackson wrote: 2) If the FileUtilsFactory is called from or implemented in FileUtils.getFileUtils() / FileUtils.newFileUtils(), every task is going to benefit. I think the implementation I provided does precisely this, or rather it calls from the interface (FileUtilsAdapter) instead of from the implementation. If you mean that the current FileUtils should delegate to another implementation using the factory that's different. I think it would be cleaner to gradually transition all the tasks over to an interface (while retaining the FileUtils semi-singleton). At the point where all the tasks are using FileUtilsAdapter, we can refactor and remove the parts of FileUtils that are exposed correctly in the interface, maintaining bwc that way - the only thing left would be a small stub which contains the static methods - that's my view of how a transition would work anyway. I think that using FileUtils to delegate/proxy to a real class would be less 'elegant' than using an interface - although I can see the immediate advantages of using the approach you mentioned (all tasks benefit without actually having to change each class over to the interface). In that case I would like some time to think things over, as to provide one face, and to have different implementations for currently different parts. Classes that support copying, things that support permissions, things that support path manipulation, without getting a really big number of classes. FileUtils for Java 1.6 on Windows FileUtils for Java 1.4 on VMS etc. I had an idea to turn FileUtils into a Facade that delegates different actions to different back-ends. Thanks for your elaborate reaction. - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: [patch] NioFileUtils, Java6FileUtils, FileUtilsAdapter + factory, build.xml (was Re: AW: Adding a methof to StringUtils)
Martijn Kruithof wrote: Hi, Adding setting the property using -D to ANT_OPTS should do. 5 things: 1) I saw you removed public members (constants) from FileUtils (Non backward compatible change, keeping everything BC also frustrates me sometimes, but still I think it is best not to break parts we do not know of.) Yes that was a mistake, I thought that they'd be available from the interface, but realised after I sent the email that only Copy is using the interface and all other tasks are still tied to the implementation. I need to put this back in the default impl to retain bwc as you mentioned. 2) If the FileUtilsFactory is called from or implemented in FileUtils.getFileUtils() / FileUtils.newFileUtils(), every task is going to benefit. I think the implementation I provided does precisely this, or rather it calls from the interface (FileUtilsAdapter) instead of from the implementation. If you mean that the current FileUtils should delegate to another implementation using the factory that's different. I think it would be cleaner to gradually transition all the tasks over to an interface (while retaining the FileUtils semi-singleton). At the point where all the tasks are using FileUtilsAdapter, we can refactor and remove the parts of FileUtils that are exposed correctly in the interface, maintaining bwc that way - the only thing left would be a small stub which contains the static methods - that's my view of how a transition would work anyway. I think that using FileUtils to delegate/proxy to a real class would be less 'elegant' than using an interface - although I can see the immediate advantages of using the approach you mentioned (all tasks benefit without actually having to change each class over to the interface). 3) Again / Still the actual copying has been moved to the ResourceUtils, how do we make sure that files addressed as resources also benefit from this change. I'm not sure what you mean by this - the code is proof of concept to show that it would be possible to add in nio based file handling in a way that minimizes problems of bwc. I didn't attempt to add nio for handling resources - indeed that would mean changing the ResourceUtils class, which I didn't want to do as it would have been a big waste of time if the overall strategy was wrong. Right now I just want to play with some basic ideas and see if people here on dev list thing that the strategy is good/hopelessly broken/not worth pursuing The intention isn't to be a complete solution to all resource handling, just to be complete enough to provide a background for further discussion - I fully expect most of the code to be thrown away, but without something that executes it's hard to see the potential flaws. Regarding the changes in the Copy task: 4) Stringbuffer is not more efficient than concatenation using + in a single statement. [snip] Sorry this was change that I didn't roll-back from earlier, it shouldn't be there 5) StringBuffer handling is incorrect for the second part, (you will concatenate all to filenames.) again please dis-regard this part of the code as it isn't meant to be there This shows what happens when you have several different ideas being worked on at the same time and you create a unified diff without looking! (my bad) Thanks for the comments Kev - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: [patch] NioFileUtils, Java6FileUtils, FileUtilsAdapter + factory, build.xml (was Re: AW: Adding a methof to StringUtils)
Hi,
Adding setting the property using -D to ANT_OPTS should do.
5 things:
1) I saw you removed public members (constants) from FileUtils (Non
backward compatible change, keeping everything BC also frustrates me
sometimes, but still I think it is best not to break parts we do not
know of.)
2) If the FileUtilsFactory is called from or implemented in
FileUtils.getFileUtils() / FileUtils.newFileUtils(), every task is going
to benefit.
3) Again / Still the actual copying has been moved to the ResourceUtils,
how do we make sure that files addressed as resources also benefit from
this change.
Regarding the changes in the Copy task:
4) Stringbuffer is not more efficient than concatenation using + in a
single statement.
-log("Copying " + fileCopyMap.size()
-+ " file" + (fileCopyMap.size() == 1 ? "" : "s")
-+ " to " + destDir.getAbsolutePath());
+StringBuffer sb = new StringBuffer("Copying ")
+.append(fileCopyMap.size())
+.append(" file")
+.append((fileCopyMap.size() == 1 ? "" : "s"))
+.append(" to ")
+.append(destDir.getAbsolutePath());
Why are you replacing the log statmenent with the StringBuffer.
It is less readable, and equivalent.
5) StringBuffer handling is incorrect for the second part, (you will
concatenate all to filenames.)
Martijn
-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
[patch] NioFileUtils, Java6FileUtils, FileUtilsAdapter + factory, build.xml (was Re: AW: Adding a methof to StringUtils)
Ok, this is what I've managed to bash together this morning:
- NioFileUtils (uses Java1.4 NIO libraries, altered the TRANSFER_SIZE as
suggested to 32000, currently no way to override that (yet))
- Java6FileUtils (currently has no implementation, but could be used in
future for implementing Java6 features (file permisions etc))
- FileUtils (refactored to imlement interface, still needs to have all
comments refactored into @see -> interface)
- FileUtilsAdapter (interface of FileUtils)
- FileUtilsFactory (uses runtime class-loading to select correct
version, also allows for selection of impl via system property or ant
property (project.getProperty), as suggested, caches the instance of the
FileUtilsAdapter on first use. This assumes that during a build a user
won't want to swap out FilUtils implementations.)
- build.xml (contains the selector code for [Nio|Java6]FileUtils)
Current problems:
I cannot get the current code to select the implementation based on the
property I'm specifying - (I'm using .ant.properties and I've also tried
-D style props). Any hint as to how this mechanism works in general
would be appreciated.
Code is just in util package, it would be nicer in it's own separate
package - open to suggestions on this.
Not sure that the selector in the build is working - after bootstrap +
build, the ant-nodeps.jar doesn't contain the Nio or the Java6FileUtils
- should they be here?
Patch attached for consideration and comments.
Btw, swapping to NIO does help the AppFuse build to get past the copying
- it now fails on HtmlUnit - which we can't do much about. So
(anecdotally anyway), NIO seems to use less memory than traditional IO.
Thanks
Kev
Index:
D:/java_projects/ant-core-trunk/src/main/org/apache/tools/ant/MagicNames.java
===
---
D:/java_projects/ant-core-trunk/src/main/org/apache/tools/ant/MagicNames.java
(revision 395527)
+++
D:/java_projects/ant-core-trunk/src/main/org/apache/tools/ant/MagicNames.java
(working copy)
@@ -127,6 +127,14 @@
* Value: [EMAIL PROTECTED]
*/
public static final String REGEXP_IMPL = "ant.regexp.regexpimpl";
+
+/**
+ * property for file utils implementation.
+ * default to classic/original
+ * @since Ant 1.7
+ * Value: [EMAIL PROTECTED]
+ */
+public static final String FILEUTILS_IMPL = "ant.util.fileutilsimpl";
/**
* property that provides the default value for javac's and
Index:
D:/java_projects/ant-core-trunk/src/main/org/apache/tools/ant/taskdefs/Copy.java
===
---
D:/java_projects/ant-core-trunk/src/main/org/apache/tools/ant/taskdefs/Copy.java
(revision 395527)
+++
D:/java_projects/ant-core-trunk/src/main/org/apache/tools/ant/taskdefs/Copy.java
(working copy)
@@ -28,25 +28,28 @@
import java.util.List;
import java.util.Map;
import java.util.Vector;
-import org.apache.tools.ant.Task;
-import org.apache.tools.ant.Project;
+
import org.apache.tools.ant.BuildException;
import org.apache.tools.ant.DirectoryScanner;
-import org.apache.tools.ant.types.Mapper;
+import org.apache.tools.ant.Project;
+import org.apache.tools.ant.Task;
import org.apache.tools.ant.types.FileSet;
+import org.apache.tools.ant.types.FilterChain;
import org.apache.tools.ant.types.FilterSet;
-import org.apache.tools.ant.types.FilterChain;
import org.apache.tools.ant.types.FilterSetCollection;
+import org.apache.tools.ant.types.Mapper;
import org.apache.tools.ant.types.Resource;
import org.apache.tools.ant.types.ResourceCollection;
import org.apache.tools.ant.types.ResourceFactory;
import org.apache.tools.ant.types.resources.FileResource;
+import org.apache.tools.ant.util.FileNameMapper;
import org.apache.tools.ant.util.FileUtils;
-import org.apache.tools.ant.util.FileNameMapper;
+import org.apache.tools.ant.util.FileUtilsAdapter;
+import org.apache.tools.ant.util.FileUtilsFactory;
+import org.apache.tools.ant.util.FlatFileNameMapper;
import org.apache.tools.ant.util.IdentityMapper;
import org.apache.tools.ant.util.ResourceUtils;
import org.apache.tools.ant.util.SourceFileScanner;
-import org.apache.tools.ant.util.FlatFileNameMapper;
/**
* Copies a file or directory to a new file
@@ -85,7 +88,7 @@
protected Hashtable completeDirMap = new Hashtable();
protected Mapper mapperElement = null;
-protected FileUtils fileUtils;
+protected FileUtilsAdapter fileUtils;
private Vector filterChains = new Vector();
private Vector filterSets = new Vector();
private String inputEncoding = null;
@@ -96,7 +99,7 @@
* Copy task constructor.
*/
public Copy() {
-fileUtils = FileUtils.getFileUtils();
+fileUtils = FileUtilsFactory.getFileUtils();
granularity = fileUtils.getFileTimestampGranularity();
}
@@ -105,7 +108,7 @@
* @return the fileutils object.
*/
protected F
Re: [patch] NioFileUtils, FileUtilsAdapter + factory (was Re: AW: Adding a methof to StringUtils)
> You do not gain by using nio when copying files to a local drive. As far as I know, you do gain a little, in the flexibility offered (memory mapped files, channels etc). The code is also (usually) much easier to maintain compared to standard IO. > > I do not know what in the implementation of nio allows to gain this > performance. When I first looked at it, I think it had something to do with the blocking behaviour of standard IO, NIO is actually Non-(blocking) IO in this respect. Sort of how Apache 1 (used blocking threads), -> Apache 2 (uses non-blocking model), at least I think that's how the performance is achieved. - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: [patch] NioFileUtils, FileUtilsAdapter + factory (was Re: AW: Adding a methof to StringUtils)
> I'm afraid I've never studied nio in much detail... do > its performance enhancements extend to non-file-based > IO? > > -Matt Hello Matt, I never studied nio in detail either. What I found out empirically in a Windows environment is that copying files to a share goes about 4 times faster when using nio compared to the traditional method. You do not gain by using nio when copying files to a local drive. I do not know what in the implementation of nio allows to gain this performance. Regards, Antoine - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: [patch] NioFileUtils, FileUtilsAdapter + factory (was Re: AW: Adding a methof to StringUtils)
--- Martijn Kruithof <[EMAIL PROTECTED]> wrote: > Matt Benson schreef: > > >viewcvs(svn) shows I did it 6.5 months ago... > >apparently so. Speaking of which, Kev (I think) > >mentioned these are static in ResourceUtils, but as > >they are unreleased we could change them to > instance > >methods. The behavior there is fairly > >straightforward, though, so I'm not sure if > anything > >would be gained by this... > > > >-Matt > > > > > Whell it would mean that if we do it in FileUtils, > we wouldn't benfit > from it when Resources come into play. Futhermore, > if introduce the nio > copy under the responsibility of the ResourceUtils > all users of > FileUtils would benefit right away. I'm afraid I've never studied nio in much detail... do its performance enhancements extend to non-file-based IO? -Matt > > Martijn > > - > To unsubscribe, e-mail: > [EMAIL PROTECTED] > For additional commands, e-mail: > [EMAIL PROTECTED] > > __ Do You Yahoo!? Tired of spam? Yahoo! Mail has the best spam protection around http://mail.yahoo.com - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: [patch] NioFileUtils, FileUtilsAdapter + factory (was Re: AW: Adding a methof to StringUtils)
Matt Benson schreef: viewcvs(svn) shows I did it 6.5 months ago... apparently so. Speaking of which, Kev (I think) mentioned these are static in ResourceUtils, but as they are unreleased we could change them to instance methods. The behavior there is fairly straightforward, though, so I'm not sure if anything would be gained by this... -Matt Whell it would mean that if we do it in FileUtils, we wouldn't benfit from it when Resources come into play. Futhermore, if introduce the nio copy under the responsibility of the ResourceUtils all users of FileUtils would benefit right away. Martijn - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: [patch] NioFileUtils, FileUtilsAdapter + factory (was Re: AW: Adding a methof to StringUtils)
--- Martijn Kruithof <[EMAIL PROTECTED]> wrote: > Matt Benson schreef: > > >Another thought about the factory--it should cache > an > >instance of each FileUtils type to minimize object > >creation. We can either just have it be known that > >FileUtilsAdapter impls should be stateless or have > a > >StatelessFileUtilsAdapter interface--if > implemented, > >cache, else don't. > > > >thoughts? > > > >-Matt > > > Well I am certainly in favour of keeping FileUtils > stateless, by the My point was only that we can't really enforce that FileUtilsAdapter (or whatever) impls are stateless. If we are making this factory-based we could allow users to (theoretically) provide their own implementation. If such an impl was stateful/non-threadsafe, singletons would misbehave. That's where my idea of a subinterface was coming from. Anyway... > way, didn't you move copying of files to > ResourceUtils some time ago? > viewcvs(svn) shows I did it 6.5 months ago... apparently so. Speaking of which, Kev (I think) mentioned these are static in ResourceUtils, but as they are unreleased we could change them to instance methods. The behavior there is fairly straightforward, though, so I'm not sure if anything would be gained by this... -Matt > Martijn > > - > To unsubscribe, e-mail: > [EMAIL PROTECTED] > For additional commands, e-mail: > [EMAIL PROTECTED] > > __ Do You Yahoo!? Tired of spam? Yahoo! Mail has the best spam protection around http://mail.yahoo.com - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: [patch] NioFileUtils, FileUtilsAdapter + factory (was Re: AW: Adding a methof to StringUtils)
Matt Benson schreef: Another thought about the factory--it should cache an instance of each FileUtils type to minimize object creation. We can either just have it be known that FileUtilsAdapter impls should be stateless or have a StatelessFileUtilsAdapter interface--if implemented, cache, else don't. thoughts? -Matt Well I am certainly in favour of keeping FileUtils stateless, by the way, didn't you move copying of files to ResourceUtils some time ago? Martijn - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: [patch] NioFileUtils, FileUtilsAdapter + factory (was Re: AW: Adding a methof to StringUtils)
O by the way I am / was looking if FileUtils could be split into os dependend version this way, so I actually did some work on this as well. Martijn Kruithof schreef: I actually had a similar problem in my dtd work recently. We had a class that applied one single strategy to determine something important for telecommunication systems, had some static methods, some nonstatic methods, was used by different components of which some could not be updated. Backward compatibility was crucial, but at the same time it was important that every component would use the correct strategy. What I did was turn the constructor into an factory (however still staying a constructor, just creating the destination class), and turn the class into a proxy, towards several implementations, one of which would be the singleton actually used (one of which having the original implementation in nonstatic methods only). Maybe the same way can be used to replace the FileUtils as wel, make the FileUtils class a Proxy with built in Factory. Martijn - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED] - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: [patch] NioFileUtils, FileUtilsAdapter + factory (was Re: AW: Adding a methof to StringUtils)
I actually had a similar problem in my dtd work recently. We had a class that applied one single strategy to determine something important for telecommunication systems, had some static methods, some nonstatic methods, was used by different components of which some could not be updated. Backward compatibility was crucial, but at the same time it was important that every component would use the correct strategy. What I did was turn the constructor into an factory (however still staying a constructor, just creating the destination class), and turn the class into a proxy, towards several implementations, one of which would be the singleton actually used (one of which having the original implementation in nonstatic methods only). Maybe the same way can be used to replace the FileUtils as wel, make the FileUtils class a Proxy with built in Factory. Martijn - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: [patch] NioFileUtils, FileUtilsAdapter + factory (was Re: AW: Adding a methof to StringUtils)
Matt Benson wrote: Another thought about the factory--it should cache an instance of each FileUtils type to minimize object creation. We can either just have it be known that FileUtilsAdapter impls should be stateless or have a StatelessFileUtilsAdapter interface--if implemented, cache, else don't. I only know that the existing FileUtils implementations are stateless. Maybe we can simply assume that this will be the case in the future. Regards, Antoine thoughts? -Matt - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: [patch] NioFileUtils, FileUtilsAdapter + factory (was Re: AW: Adding a methof to StringUtils)
Another thought about the factory--it should cache an instance of each FileUtils type to minimize object creation. We can either just have it be known that FileUtilsAdapter impls should be stateless or have a StatelessFileUtilsAdapter interface--if implemented, cache, else don't. thoughts? -Matt --- Matt Benson <[EMAIL PROTECTED]> wrote: > --- Kevin Jackson <[EMAIL PROTECTED]> wrote: > > > > I found it in Kev's (long :) mail: > > > > > > > Sorry it was rather long, but there were a few > files > > packed in there! > > parenthetical smiley above. > > > > Since the other impls will be conditionally > > compiled, > > > we should use Class.forInstance()... > > > > > > > ok, so we use dynamic class-loading to get the > > correct fileutils? Is > > there an example in the code already of how I > should > > do this, (like in > > the ComplilerAdapter code perhaps?) > > > That's the way I understand it. Antoine cited > Regexp > factory stuff (o.a.t.a.util.regexp); it looks like > the > biggest lesson to take from here would be the use of > ClasspathUtils: > > return (FileUtilsAdapter) > ClasspathUtils.newInstance(className, > FileUtilsAdapter.class.getClassLoader(), > FileUtilsAdapter.class); > > -Matt > > [SNIP] > > __ > Do You Yahoo!? > Tired of spam? Yahoo! Mail has the best spam > protection around > http://mail.yahoo.com > > - > To unsubscribe, e-mail: > [EMAIL PROTECTED] > For additional commands, e-mail: > [EMAIL PROTECTED] > > __ Do You Yahoo!? Tired of spam? Yahoo! Mail has the best spam protection around http://mail.yahoo.com - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: [patch] NioFileUtils, FileUtilsAdapter + factory (was Re: AW: Adding a methof to StringUtils)
--- Kevin Jackson <[EMAIL PROTECTED]> wrote: > > I found it in Kev's (long :) mail: > > > > Sorry it was rather long, but there were a few files > packed in there! parenthetical smiley above. > > Since the other impls will be conditionally > compiled, > > we should use Class.forInstance()... > > > > ok, so we use dynamic class-loading to get the > correct fileutils? Is > there an example in the code already of how I should > do this, (like in > the ComplilerAdapter code perhaps?) > That's the way I understand it. Antoine cited Regexp factory stuff (o.a.t.a.util.regexp); it looks like the biggest lesson to take from here would be the use of ClasspathUtils: return (FileUtilsAdapter) ClasspathUtils.newInstance(className, FileUtilsAdapter.class.getClassLoader(), FileUtilsAdapter.class); -Matt [SNIP] __ Do You Yahoo!? Tired of spam? Yahoo! Mail has the best spam protection around http://mail.yahoo.com - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: [patch] NioFileUtils, FileUtilsAdapter + factory (was Re: AW: Adding a methof to StringUtils)
Kevin Jackson wrote:
I found it in Kev's (long :) mail:
Sorry it was rather long, but there were a few files packed in there!
No problem, I should have washed my glasses this morning, ...
Since the other impls will be conditionally compiled,
we should use Class.forInstance()...
ok, so we use dynamic class-loading to get the correct fileutils? Is
there an example in the code already of how I should do this, (like in
the ComplilerAdapter code perhaps?)
in the RegexpFactory there is an example.
What would the Project reference be used for?
in the RegexpFactory, a project reference is used so that properties can
be accessed, and that the user can direct ant to use a particular
implementation.
Kev: your mail showed:
public class NioFileUtils extends FileUtils implements
FileUtilsAdapter {...}
public class Java6FileUtils extends FileUtils
implements FileUtilsAdapter {...}
But shouldn't that actually be:
+public class FileUtils implements FileUtilsAdapter
{...}
public class NioFileUtils extends FileUtils {...}
public class Java6FileUtils extends NioFileUtils {...}
Probably yes, I think I cobbled it together in a hap-hazard fashion -
first the interface, then change the FileUtils etc, so I could
probably remove the implements FileUtilsAdapter from the subclasses, I
just wanted to make sure I wasn't 'polishing a turd', ie wasting time
getting every little thing right, if the overall direction of the code
was wrong.
A last thing : I think that the default buffer size (variable
TRANSFER_SIZE) should be reduced from 81920 to 32000 and should be
configurable.
I used this nio copy before, and larger transfer sizes were sometimes
creating exceptions (error messages coming from the operating system).
Thanks for comments
Kev
Antoine
-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
Re: [patch] NioFileUtils, FileUtilsAdapter + factory (was Re: AW: Adding a methof to StringUtils)
> I found it in Kev's (long :) mail:
>
Sorry it was rather long, but there were a few files packed in there!
> Since the other impls will be conditionally compiled,
> we should use Class.forInstance()...
>
ok, so we use dynamic class-loading to get the correct fileutils? Is
there an example in the code already of how I should do this, (like in
the ComplilerAdapter code perhaps?)
>
> What would the Project reference be used for?
>
> Kev: your mail showed:
>
> public class NioFileUtils extends FileUtils implements
> FileUtilsAdapter {...}
>
> public class Java6FileUtils extends FileUtils
> implements FileUtilsAdapter {...}
>
>
> But shouldn't that actually be:
>
> +public class FileUtils implements FileUtilsAdapter
> {...}
>
> public class NioFileUtils extends FileUtils {...}
>
> public class Java6FileUtils extends NioFileUtils {...}
>
Probably yes, I think I cobbled it together in a hap-hazard fashion -
first the interface, then change the FileUtils etc, so I could
probably remove the implements FileUtilsAdapter from the subclasses, I
just wanted to make sure I wasn't 'polishing a turd', ie wasting time
getting every little thing right, if the overall direction of the code
was wrong.
Thanks for comments
Kev
-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
Re: [patch] NioFileUtils, FileUtilsAdapter + factory (was Re: AW: Adding a methof to StringUtils)
--- Antoine Levy-Lambert <[EMAIL PROTECTED]> wrote:
> Hello Kevin,
>
> NioFileUtils should go into another package
> org.apache.tools.ant.util.java14 otherwise we are
> going to have build
> problems under java 1.2
> This should be entered in the build.xml (selector
> needs.jdk14+)
> Therefore NioFileUtils would be packaged in
> ant-nodeps.jar
>
I don't really care either way here, but couldn't we
explicitly exclude the file(s)? Why must they live in
another directory?
> We need also a FileUtilsFactory. The
> FileUtilsFactory would instantiate
> NioFileUtils if the java runtime is 1.4 or 1.5, the
> normal FileUtils
> otherwise.
> This should be developed along the lines of the
> RegexpFactory, which
> means no imports of optional classes such as
> NioFileUtils or
> Java16FileUtils.
I found it in Kev's (long :) mail:
package org.apache.tools.ant.util;
public class FileUtilsAdapterFactory {
private FileUtilsAdapterFactory() {}
public static FileUtilsAdapter getFileUtils(final
String type) {
if (type.equals("nio")) {
//just print out to see if it's actually
picking up the correct class
System.out.println("Using nio fileutils");
return new NioFileUtils();
} else if (type.equals("java6")) {
System.out.println("Using java6
fileutils");
return new Java6FileUtils();
} else {
System.out.println("Using classic
fileutils");
return new FileUtils();
}
}
public static FileUtilsAdapter getBestFileUtils()
{
final int v =
JavaEnvUtils.getJavaVersionNumber();
if (v >= 16) {
return getFileUtils("java6");
} else if (v >=14) {
return getFileUtils("nio");
} else {
return new FileUtils();
}
}
}
Since the other impls will be conditionally compiled,
we should use Class.forInstance()...
>
> Similarly, users of FileUtils would call
> FileUtilsFactory.newFileUtils(Project p) instead of
> FileUtils.newFileUtils() where a project member
> variable is available,
> otherwise FileUtilsFactory.newFileUtils().
What would the Project reference be used for?
Kev: your mail showed:
public class NioFileUtils extends FileUtils implements
FileUtilsAdapter {...}
public class Java6FileUtils extends FileUtils
implements FileUtilsAdapter {...}
But shouldn't that actually be:
+public class FileUtils implements FileUtilsAdapter
{...}
public class NioFileUtils extends FileUtils {...}
public class Java6FileUtils extends NioFileUtils {...}
?
br,
Matt
__
Do You Yahoo!?
Tired of spam? Yahoo! Mail has the best spam protection around
http://mail.yahoo.com
-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
Re: [patch] NioFileUtils, FileUtilsAdapter + factory (was Re: AW: Adding a methof to StringUtils)
Hello Kevin,
NioFileUtils should go into another package
org.apache.tools.ant.util.java14 otherwise we are going to have build
problems under java 1.2
This should be entered in the build.xml (selector needs.jdk14+)
Therefore NioFileUtils would be packaged in ant-nodeps.jar
We need also a FileUtilsFactory. The FileUtilsFactory would instantiate
NioFileUtils if the java runtime is 1.4 or 1.5, the normal FileUtils
otherwise.
This should be developed along the lines of the RegexpFactory, which
means no imports of optional classes such as NioFileUtils or
Java16FileUtils.
Similarly, users of FileUtils would call
FileUtilsFactory.newFileUtils(Project p) instead of
FileUtils.newFileUtils() where a project member variable is available,
otherwise FileUtilsFactory.newFileUtils().
FileUtilsFactory would be the place where the right FileUtils
implementation is picked.
Regards,
Antoine
Kev Jackson wrote:
> Hi,
>
> As promised, here's the code that I hacked together today at Antoine's
> suggestion regarding a patch in bugzilla [1].
>
> Feel free to tear it to pieces and point out obvious problems. I've
> patched my version of copy to use this code instead of FileUtils
> directly and so far there have been no problems with it.
>
> Thanks
> Kev
>
> [1]http://issues.apache.org/bugzilla/show_bug.cgi?id=30094
>
>
> Index:
> D:/java_projects/ant-core-trunk/src/main/org/apache/tools/ant/util/FileUtils.java
> ===
> ---
> D:/java_projects/ant-core-trunk/src/main/org/apache/tools/ant/util/FileUtils.java
> (revision 395527)
> +++
> D:/java_projects/ant-core-trunk/src/main/org/apache/tools/ant/util/FileUtils.java
> (working copy)
> @@ -46,7 +46,7 @@
> * their last modification time.
> *
> */
> -public class FileUtils {
> +public class FileUtils implements FileUtilsAdapter {
>
> private static final FileUtils PRIMARY_INSTANCE = new FileUtils();
>
> Index:
> D:/java_projects/ant-core-trunk/src/main/org/apache/tools/ant/util/FileUtilsAdapter.java
> ===
> /*
> * Copyright 2006 The Apache Software Foundation
> *
> * Licensed under the Apache License, Version 2.0 (the "License");
> * you may not use this file except in compliance with the License.
> * You may obtain a copy of the License at
> *
> * http://www.apache.org/licenses/LICENSE-2.0
> *
> * Unless required by applicable law or agreed to in writing, software
> * distributed under the License is distributed on an "AS IS" BASIS,
> * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> * See the License for the specific language governing permissions and
> * limitations under the License.
> *
> */
>
> package org.apache.tools.ant.util;
>
> import java.io.File;
> import java.io.IOException;
> import java.net.MalformedURLException;
> import java.net.URL;
> import java.util.Vector;
>
> import org.apache.tools.ant.Project;
> import org.apache.tools.ant.types.FilterSetCollection;
>
> /**
> * An interface for the different types of FileUtils (classic, Nio, Java6+)
> *
> */
> public interface FileUtilsAdapter {
>
> /**
> * The granularity of timestamps under FAT.
> */
> public static final long FAT_FILE_TIMESTAMP_GRANULARITY = 2000;
>
> /**
> * The granularity of timestamps under Unix.
> */
> public static final long UNIX_FILE_TIMESTAMP_GRANULARITY = 1000;
>
> /**
> * The granularity of timestamps under the NT File System.
> * NTFS has a granularity of 100 nanoseconds, which is less
> * than 1 millisecond, so we round this up to 1 millisecond.
> */
> public static final long NTFS_FILE_TIMESTAMP_GRANULARITY = 1;
>
> /**
> * Get the URL for a file taking into account # characters.
> *
> * @param file the file whose URL representation is required.
> * @return The FileURL value.
> * @throws MalformedURLException if the URL representation cannot be
> * formed.
> */
> public URL getFileURL(File file) throws MalformedURLException;
>
> /**
> * Convenience method to copy a file from a source to a destination.
> * No filtering is performed.
> *
> * @param sourceFile Name of file to copy from.
> * Must not be null.
> * @param destFile Name of file to copy to.
> * Must not be null.
> *
> * @throws IOException if the copying fails.
> */
> public void copyFile(String sourceFile, String destFile)
> throws IOException;
>
> /**
> * Convenience method to copy a file from a source to a destination
> * specifying if token filtering must be used.
> *
> * @param sourceFile Name of file to copy from.
> * Must not be null.
> * @param destFile Name of file to c
[patch] NioFileUtils, FileUtilsAdapter + factory (was Re: AW: Adding a methof to StringUtils)
Hi,
As promised, here's the code that I hacked together today at Antoine's
suggestion regarding a patch in bugzilla [1].
Feel free to tear it to pieces and point out obvious problems. I've
patched my version of copy to use this code instead of FileUtils
directly and so far there have been no problems with it.
Thanks
Kev
[1]http://issues.apache.org/bugzilla/show_bug.cgi?id=30094
Index:
D:/java_projects/ant-core-trunk/src/main/org/apache/tools/ant/util/FileUtils.java
===
---
D:/java_projects/ant-core-trunk/src/main/org/apache/tools/ant/util/FileUtils.java
(revision 395527)
+++
D:/java_projects/ant-core-trunk/src/main/org/apache/tools/ant/util/FileUtils.java
(working copy)
@@ -46,7 +46,7 @@
* their last modification time.
*
*/
-public class FileUtils {
+public class FileUtils implements FileUtilsAdapter {
private static final FileUtils PRIMARY_INSTANCE = new FileUtils();
Index:
D:/java_projects/ant-core-trunk/src/main/org/apache/tools/ant/util/FileUtilsAdapter.java
===
/*
* Copyright 2006 The Apache Software Foundation
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*
*/
package org.apache.tools.ant.util;
import java.io.File;
import java.io.IOException;
import java.net.MalformedURLException;
import java.net.URL;
import java.util.Vector;
import org.apache.tools.ant.Project;
import org.apache.tools.ant.types.FilterSetCollection;
/**
* An interface for the different types of FileUtils (classic, Nio, Java6+)
*
*/
public interface FileUtilsAdapter {
/**
* The granularity of timestamps under FAT.
*/
public static final long FAT_FILE_TIMESTAMP_GRANULARITY = 2000;
/**
* The granularity of timestamps under Unix.
*/
public static final long UNIX_FILE_TIMESTAMP_GRANULARITY = 1000;
/**
* The granularity of timestamps under the NT File System.
* NTFS has a granularity of 100 nanoseconds, which is less
* than 1 millisecond, so we round this up to 1 millisecond.
*/
public static final long NTFS_FILE_TIMESTAMP_GRANULARITY = 1;
/**
* Get the URL for a file taking into account # characters.
*
* @param file the file whose URL representation is required.
* @return The FileURL value.
* @throws MalformedURLException if the URL representation cannot be
* formed.
*/
public URL getFileURL(File file) throws MalformedURLException;
/**
* Convenience method to copy a file from a source to a destination.
* No filtering is performed.
*
* @param sourceFile Name of file to copy from.
* Must not be null.
* @param destFile Name of file to copy to.
* Must not be null.
*
* @throws IOException if the copying fails.
*/
public void copyFile(String sourceFile, String destFile)
throws IOException;
/**
* Convenience method to copy a file from a source to a destination
* specifying if token filtering must be used.
*
* @param sourceFile Name of file to copy from.
* Must not be null.
* @param destFile Name of file to copy to.
* Must not be null.
* @param filters the collection of filters to apply to this copy.
*
* @throws IOException if the copying fails.
*/
public void copyFile(String sourceFile, String destFile,
FilterSetCollection filters)
throws IOException;
/**
* Convenience method to copy a file from a source to a
* destination specifying if token filtering must be used and if
* source files may overwrite newer destination files.
*
* @param sourceFile Name of file to copy from.
* Must not be null.
* @param destFile Name of file to copy to.
* Must not be null.
* @param filters the collection of filters to apply to this copy.
* @param overwrite Whether or not the destination file should be
* overwritten if it already exists.
*
* @throws IOException if the copying fails.
*/
public void copyFile(String sourceFile, String destFile,
FilterSetCollection filters,
boolean overwrite) throws IOException;
/**
* Convenience method to copy a file from a s
Re: AW: Adding a methof to StringUtils
Kev Jackson wrote: If it came to it I'd -1 it too! I don't like any of the solutions I could come up with yesterday, the one I showed was the 'least worst' that I could think of, with a semi-upgrade path to Java5 style varargs (use an object array). I was mainly throwing the idea out to see what peoples reactions were - overall I don't think it's the right way to solve the problem - the real problem is that the level of logging cannot currently be determined and so any optimization (for memory or performance) is actually changing the behaviour of the code - which it shouldn't do. The delete task was just an example - I was looking at it to fix 'delete task won't be quiet' bug in bugzilla, and I was also thinking about the problem with AppFuse, (which does use Delete a little, but not as much as Copy and other tasks), so it seemed like a handy guinea pig as I had the code open at the time. I was also looking at Copy and saw that ResourceUtils.copyResource is a static method, but FileUtils.copyFile is not even though it delegates to copyResource, this means that in Copy there must be an instantiated fileUtils object, just to perform the copy, unfortunately the FileUtils interface/API is public so changing it would break bwc, but I'd like to add a static method for copyFile so that Copy wont need to instantiate FileUtils. One thing I've always been curious about is how much speedup we'd get by turning off all logging. That is, if I modified log() to discard its contents, how much faster would everything be? We couldnt use the test suite as a benchmark, because too many tests depend on the log, and it probably isnt realistic. - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: AW: Adding a methof to StringUtils
Antoine Levy-Lambert wrote: Matt Benson wrote: Kev: I've asked some stuff about FileUtils before... historically it was thought it might be pluggable, hence all the instance methods. But that never happened so the static methods crept in. I think it was Martijn who really pushed in the FileUtils singleton. Most places in Ant's code don't instantiate their own FileUtils anymore, and if they do they shouldn't; they should use the singleton. -Matt Hello Kev, if you want to do some optimization work for copy, can you have a look at this bug report : http://issues.apache.org/bugzilla/show_bug.cgi?id=30094 Using nio for copy operations to network drives under Windows brings a huge performance improvement. I used it on Windows 2000 or 2003 and JDK 1.4, with as target some drives automounted from a NAS box. With NIO the copy speed is at least 4 times as big. Last time I used NIO was just when 1.4 came out, and while the speed was impressive (over 1.3), the memory usage was very large (for file mapped stuff). At the time I was processing large files, so I didn't get to play with it as it made our server (Sun something) cry :(. I've knocked up a quick implementation of what is suggested in the bug report (and Steve L's suggested strategy, at least my understanding of said strategy). Now I have a FileUtilsAdapterFactory, a FileUtilsAdapter and 3 implementations FileUtils - for bwc, NioFileUtils and Java6FileUtils. So far all is well - indeed during bootstrap, Ant handily picks up the new NioFileUtils to perform it's copies - yay! I'll post full code for comments/review later, but to implement this across the whole code base will take a while - I'm not sure everyone will think it's worthwhile. Kev - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: AW: Adding a methof to StringUtils
Matt Benson wrote: > > Kev: I've asked some stuff about FileUtils before... > historically it was thought it might be pluggable, > hence all the instance methods. But that never > happened so the static methods crept in. I think it > was Martijn who really pushed in the FileUtils > singleton. Most places in Ant's code don't > instantiate their own FileUtils anymore, and if they > do they shouldn't; they should use the singleton. > > -Matt > Hello Kev, if you want to do some optimization work for copy, can you have a look at this bug report : http://issues.apache.org/bugzilla/show_bug.cgi?id=30094 Using nio for copy operations to network drives under Windows brings a huge performance improvement. I used it on Windows 2000 or 2003 and JDK 1.4, with as target some drives automounted from a NAS box. With NIO the copy speed is at least 4 times as big. This type of progress dwarfs logging optimization. Of course, this is mainly to be used (or only) in the case when one copies 100% of the content of a file to another location, no filtering takes place. Regards, Antoine - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: AW: Adding a methof to StringUtils
--- Kev Jackson <[EMAIL PROTECTED]> wrote:
> Dominique Devienne wrote:
>
> >>The point about building the strings when they
> aren't used (because
> >>logging verbosity is set too low) still stands
> though - this is less
> >>than efficient
> >>
> >>
> >
> >The point, which Matt already raised, is that you
> can't know the level
> >at which the logger and the listeners are set.
> There is nothing ATM in
> >the Ant codebase to tip off the project as to the
> min log level of all
> >logger+listener to implement your optimization.
> This could be can be
> >added though, and would be useful IMHO, but that's
> a separate thread.
> >
> >
> >
> I know, that was why I didn't want to wrap the log
> code in if(verbosity
> >= Project.MSG_INFO) conditionals - it's sort of a
> catch-22 situation,
> you don't want to log the message (or even bother
> building it) if it
> won't be used, but that can't be determined, so you
> must build the
> message just in case - I agree a mechanism for you
> to retrieve the
> minimum logging level of the entire project would be
> useful in this
> situation
>
> >The StringBuffer optimization is well known. I've
> either read about it
> >in Effective Java or the "K&R" of Java,
> The.Java.Prog.Lang.
> >
> >
> I think it was in Effective Java, but as Alexey
> showed previously,
> Java1.4+ actually perform this operation for you,
> letting you be as lazy
> as you like with String concatenation (now that's
> progress!). I have to
> stop beating up the junior devs here whenever I see
> sqlString = "Select
> " + field + " from " + table + " where " + field + "
> = value"; as we are
> on 1.4 internally so it doesn't degrade performance
> anymore. Still I'd
> prefer to be able to do something like "yada yada
> ${var1} yada yada
> ${var2}" like the property expansion that is already
> in Ant - I
> personally think it's much cleaner than "yada yada "
> + var1 + "yada
> yada" + var2., even the Java5 printf isn't as nice
> as it could be for
> constructing messages (although it's a long way
> forward).
>
> >I think what you propose to do would clutter the
> code, and make it
> >ugly frankly ;-) I'd probably -1 it unless you can
> show hard evidence
> >of it's usefulness.
> >
> >
> If it came to it I'd -1 it too! I don't like any of
> the solutions I
> could come up with yesterday, the one I showed was
> the 'least worst'
> that I could think of, with a semi-upgrade path to
> Java5 style varargs
> (use an object array). I was mainly throwing the
> idea out to see what
> peoples reactions were - overall I don't think it's
> the right way to
> solve the problem - the real problem is that the
> level of logging cannot
> currently be determined and so any optimization (for
> memory or
> performance) is actually changing the behaviour of
> the code - which it
> shouldn't do.
>
> The delete task was just an example - I was looking
> at it to fix 'delete
> task won't be quiet' bug in bugzilla, and I was also
> thinking about the
> problem with AppFuse, (which does use Delete a
> little, but not as much
> as Copy and other tasks), so it seemed like a handy
> guinea pig as I had
> the code open at the time.
>
> I was also looking at Copy and saw that
> ResourceUtils.copyResource is a
> static method, but FileUtils.copyFile is not even
> though it delegates to
> copyResource, this means that in Copy there must be
> an instantiated
> fileUtils object, just to perform the copy,
> unfortunately the FileUtils
> interface/API is public so changing it would break
> bwc, but I'd like to
> add a static method for copyFile so that Copy wont
> need to instantiate
> FileUtils.
Kev: I've asked some stuff about FileUtils before...
historically it was thought it might be pluggable,
hence all the instance methods. But that never
happened so the static methods crept in. I think it
was Martijn who really pushed in the FileUtils
singleton. Most places in Ant's code don't
instantiate their own FileUtils anymore, and if they
do they shouldn't; they should use the singleton.
-Matt
>
> Thanks everyone for feedback
> Kev
>
>
-
> To unsubscribe, e-mail:
> [EMAIL PROTECTED]
> For additional commands, e-mail:
> [EMAIL PROTECTED]
>
>
__
Do You Yahoo!?
Tired of spam? Yahoo! Mail has the best spam protection around
http://mail.yahoo.com
-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
Re: AW: Adding a methof to StringUtils
Dominique Devienne wrote:
The point about building the strings when they aren't used (because
logging verbosity is set too low) still stands though - this is less
than efficient
The point, which Matt already raised, is that you can't know the level
at which the logger and the listeners are set. There is nothing ATM in
the Ant codebase to tip off the project as to the min log level of all
logger+listener to implement your optimization. This could be can be
added though, and would be useful IMHO, but that's a separate thread.
I know, that was why I didn't want to wrap the log code in if(verbosity
>= Project.MSG_INFO) conditionals - it's sort of a catch-22 situation,
you don't want to log the message (or even bother building it) if it
won't be used, but that can't be determined, so you must build the
message just in case - I agree a mechanism for you to retrieve the
minimum logging level of the entire project would be useful in this
situation
The StringBuffer optimization is well known. I've either read about it
in Effective Java or the "K&R" of Java, The.Java.Prog.Lang.
I think it was in Effective Java, but as Alexey showed previously,
Java1.4+ actually perform this operation for you, letting you be as lazy
as you like with String concatenation (now that's progress!). I have to
stop beating up the junior devs here whenever I see sqlString = "Select
" + field + " from " + table + " where " + field + " = value"; as we are
on 1.4 internally so it doesn't degrade performance anymore. Still I'd
prefer to be able to do something like "yada yada ${var1} yada yada
${var2}" like the property expansion that is already in Ant - I
personally think it's much cleaner than "yada yada " + var1 + "yada
yada" + var2., even the Java5 printf isn't as nice as it could be for
constructing messages (although it's a long way forward).
I think what you propose to do would clutter the code, and make it
ugly frankly ;-) I'd probably -1 it unless you can show hard evidence
of it's usefulness.
If it came to it I'd -1 it too! I don't like any of the solutions I
could come up with yesterday, the one I showed was the 'least worst'
that I could think of, with a semi-upgrade path to Java5 style varargs
(use an object array). I was mainly throwing the idea out to see what
peoples reactions were - overall I don't think it's the right way to
solve the problem - the real problem is that the level of logging cannot
currently be determined and so any optimization (for memory or
performance) is actually changing the behaviour of the code - which it
shouldn't do.
The delete task was just an example - I was looking at it to fix 'delete
task won't be quiet' bug in bugzilla, and I was also thinking about the
problem with AppFuse, (which does use Delete a little, but not as much
as Copy and other tasks), so it seemed like a handy guinea pig as I had
the code open at the time.
I was also looking at Copy and saw that ResourceUtils.copyResource is a
static method, but FileUtils.copyFile is not even though it delegates to
copyResource, this means that in Copy there must be an instantiated
fileUtils object, just to perform the copy, unfortunately the FileUtils
interface/API is public so changing it would break bwc, but I'd like to
add a static method for copyFile so that Copy wont need to instantiate
FileUtils.
Thanks everyone for feedback
Kev
-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
RE: AW: Adding a methof to StringUtils
--- "Burgess, Benjamin" <[EMAIL PROTECTED]> wrote: > A good description of this comes from the JavaDocs > for Log4j here: > > http://logging.apache.org/log4j/docs/api/org/apache/log4j/Category.html# > isDebugEnabled() > > So actually, you would only wrap the debug log > statement in an > if(log.isDebug()) if the message itself is more than > a simple String > (ie. Contains concatenation or creates an object). > It might be useful to remember this... the main problem in Ant, though, is that a Project doesn't currently know at what level it is being logged. Only its listeners know that. -Matt > Ben [SNIP] __ Do You Yahoo!? Tired of spam? Yahoo! Mail has the best spam protection around http://mail.yahoo.com - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
RE: AW: Adding a methof to StringUtils
A good description of this comes from the JavaDocs for Log4j here:
http://logging.apache.org/log4j/docs/api/org/apache/log4j/Category.html#
isDebugEnabled()
So actually, you would only wrap the debug log statement in an
if(log.isDebug()) if the message itself is more than a simple String
(ie. Contains concatenation or creates an object).
Ben
-Original Message-
From: Steve Loughran [mailto:[EMAIL PROTECTED]
Sent: Wednesday, April 19, 2006 8:49 AM
To: Ant Developers List
Subject: Re: AW: Adding a methof to StringUtils
Kev Jackson wrote:
> [EMAIL PROTECTED] wrote:
>
>>> As I've been hacking away today, I've been swapping a load of "" +
""
>>> + "" style code over to use StringBuffers. I thought that perhaps
>>> there's a potential use of a static method in StringUtils to
>>> construct these strings from an array
>>>
>>
>>
>> For a significant performance boost we had to refactor the whole (or
a
>> major part) of the codebase in this manner. So I think a performance
>> test would be good. How much is the improvement on ?
>>
>>
>>
>>
> It's less to do with speed performance and more to do with memory
> performance. "a" + "b" + "c" creates "a" + "bc" <- intermediate
String,
> and then "abc"
>
> I'd love to do a performance/memory test of some kind, perhaps I'll
use
> the new NetBeans (it's supposed to have a good view of the heap being
> used). Any suggestions on tools to help evaluate changes would be
helpful.
>
> We also have a problem with logging in general where the Strings are
> concatenated :
> log("a" + toFile + " b " + dir + etc, verbosity);
>
> This will produce a lot of temp Strings, send the result to log and
> crucially, if the verbosity is too low, won't even use it - that's a
> waste of memory in every sense of the word. I'd like to fix this, the
> appendMessage method I proposed is only a bandaid for now.
>
> Essentially we don't want to perform any operations, or produce any
> temporary values if they aren't going to be used, but I can't see a
way
> of doing this without something like
>
> if (verbosity >= Project.MSG_INFO) {
> log(msg + e + msg2);
> }
>
> but this is subverting the log method that uses the verbosity
> internally, and adding conditions to the construction of log messages
is
> not nice.
This is the pattern you encounter with log4j/commons-logging a lot
if(log.isDebug() {
log.debug("something);
}
skipping the entire concat and log process would speed things up. Too
bad the log interface spews everything out and then it gets discarded
later. I think in the Ant1.8+ timeframe we could think about tuning how
we log to let tasks find out more about how they get logged and feed
that back in to string creation.
Regarding the change, I thought for a moment you were in Java5 varargs
code. Whatever changes are used they should ideally prepare ant for a
world where varargs logging is possible, some years down the line, or
much closer in third party java5+ only tasks.
-steve
-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
**
This message, including any attachments, contains confidential information
intended for a specific individual and purpose, and is protected by law. If
you are not the intended recipient, please contact sender immediately by reply
e-mail and destroy all copies. You are hereby notified that any disclosure,
copying, or distribution of this message, or the taking of any action based on
it, is strictly prohibited.
TIAA-CREF
**
-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
Re: AW: Adding a methof to StringUtils
--- Dominique Devienne <[EMAIL PROTECTED]> wrote: > > The point about building the strings when they > aren't used (because > > logging verbosity is set too low) still stands > though - this is less > > than efficient > > The point, which Matt already raised, is that you > can't know the level > at which the logger and the listeners are set. There Actually, I think that was Jan. :) > is nothing ATM in > the Ant codebase to tip off the project as to the > min log level of all > logger+listener to implement your optimization. This > could be can be > added though, and would be useful IMHO, but that's a > separate thread. > > The StringBuffer optimization is well known. I've > either read about it > in Effective Java or the "K&R" of Java, > The.Java.Prog.Lang. > > I think what you propose to do would clutter the > code, and make it > ugly frankly ;-) I'd probably -1 it unless you can > show hard evidence > of it's usefulness. > > If is causing problem, replacing it by a > custom task which is > streamlined and doesn't need to buffer all files to > delete beforehand > for example. --DD > I had tried to make do as little buffering as possible. The original resourceCollection-enabled version was reported by Alexey to present a major slowdown for existing large fileset deletions, but I refactored the fileset handling to try to avoid this. At present I can see where more speed could possibly be coaxed out here, but it would cost memory which is apparently already a problem. Any relevant profiling results would be of interest to me. -Matt > - > To unsubscribe, e-mail: > [EMAIL PROTECTED] > For additional commands, e-mail: > [EMAIL PROTECTED] > > __ Do You Yahoo!? Tired of spam? Yahoo! Mail has the best spam protection around http://mail.yahoo.com - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: AW: Adding a methof to StringUtils
> The point about building the strings when they aren't used (because > logging verbosity is set too low) still stands though - this is less > than efficient The point, which Matt already raised, is that you can't know the level at which the logger and the listeners are set. There is nothing ATM in the Ant codebase to tip off the project as to the min log level of all logger+listener to implement your optimization. This could be can be added though, and would be useful IMHO, but that's a separate thread. The StringBuffer optimization is well known. I've either read about it in Effective Java or the "K&R" of Java, The.Java.Prog.Lang. I think what you propose to do would clutter the code, and make it ugly frankly ;-) I'd probably -1 it unless you can show hard evidence of it's usefulness. If is causing problem, replacing it by a custom task which is streamlined and doesn't need to buffer all files to delete beforehand for example. --DD - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: AW: Adding a methof to StringUtils
Kev Jackson wrote:
[EMAIL PROTECTED] wrote:
As I've been hacking away today, I've been swapping a load of "" + ""
+ "" style code over to use StringBuffers. I thought that perhaps
there's a potential use of a static method in StringUtils to
construct these strings from an array
For a significant performance boost we had to refactor the whole (or a
major part) of the codebase in this manner. So I think a performance
test would be good. How much is the improvement on ?
It's less to do with speed performance and more to do with memory
performance. "a" + "b" + "c" creates "a" + "bc" <- intermediate String,
and then "abc"
I'd love to do a performance/memory test of some kind, perhaps I'll use
the new NetBeans (it's supposed to have a good view of the heap being
used). Any suggestions on tools to help evaluate changes would be helpful.
We also have a problem with logging in general where the Strings are
concatenated :
log("a" + toFile + " b " + dir + etc, verbosity);
This will produce a lot of temp Strings, send the result to log and
crucially, if the verbosity is too low, won't even use it - that's a
waste of memory in every sense of the word. I'd like to fix this, the
appendMessage method I proposed is only a bandaid for now.
Essentially we don't want to perform any operations, or produce any
temporary values if they aren't going to be used, but I can't see a way
of doing this without something like
if (verbosity >= Project.MSG_INFO) {
log(msg + e + msg2);
}
but this is subverting the log method that uses the verbosity
internally, and adding conditions to the construction of log messages is
not nice.
This is the pattern you encounter with log4j/commons-logging a lot
if(log.isDebug() {
log.debug("something);
}
skipping the entire concat and log process would speed things up. Too
bad the log interface spews everything out and then it gets discarded
later. I think in the Ant1.8+ timeframe we could think about tuning how
we log to let tasks find out more about how they get logged and feed
that back in to string creation.
Regarding the change, I thought for a moment you were in Java5 varargs
code. Whatever changes are used they should ideally prepare ant for a
world where varargs logging is possible, some years down the line, or
much closer in third party java5+ only tasks.
-steve
-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
Re: AW: Adding a methof to StringUtils
javap -c AAA public java.lang.String concat(java.lang.String, java.lang.String, java.lang.String); Code: 0: new #2; //class java/lang/StringBuilder 3: dup 4: invokespecial #3; //Method java/lang/StringBuilder."":()V 7: aload_1 8: invokevirtual #4; //Method java/lang/StringBuilder.append:(Ljava/lang/String;)Ljava/lang/StringBuilder; 11: aload_2 12: invokevirtual #4; //Method java/lang/StringBuilder.append:(Ljava/lang/String;)Ljava/lang/StringBuilder; 15: aload_3 16: invokevirtual #4; //Method java/lang/StringBuilder.append:(Ljava/lang/String;)Ljava/lang/StringBuilder; 19: invokevirtual #5; //Method java/lang/StringBuilder.toString:()Ljava/lang/String; 22: areturn [snip 1.4 output] Thanks for showing me this, it's time I re-read the JVM spec I suppose as it's obviously been updated! It is strange though that by making changes to an explicit StringBuffer instead of using String concatenation, the memory usage of the AppFuse build was reduced enough to avoid an OutOfMemory error. The point about building the strings when they aren't used (because logging verbosity is set too low) still stands though - this is less than efficient Thanks Kev - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: AW: Adding a methof to StringUtils
[EMAIL PROTECTED] wrote:
As I've been hacking away today, I've been swapping a load of
"" + "" + "" style code over to use StringBuffers. I thought
that perhaps there's a potential use of a static method in
StringUtils to construct these strings from an array
For a significant performance boost we had to refactor the whole (or a
major part) of the codebase in this manner. So I think a performance
test would be good. How much is the improvement on ?
It's less to do with speed performance and more to do with memory
performance. "a" + "b" + "c" creates "a" + "bc" <- intermediate String,
and then "abc"
I'd love to do a performance/memory test of some kind, perhaps I'll use
the new NetBeans (it's supposed to have a good view of the heap being
used). Any suggestions on tools to help evaluate changes would be helpful.
We also have a problem with logging in general where the Strings are
concatenated :
log("a" + toFile + " b " + dir + etc, verbosity);
This will produce a lot of temp Strings, send the result to log and
crucially, if the verbosity is too low, won't even use it - that's a
waste of memory in every sense of the word. I'd like to fix this, the
appendMessage method I proposed is only a bandaid for now.
Essentially we don't want to perform any operations, or produce any
temporary values if they aren't going to be used, but I can't see a way
of doing this without something like
if (verbosity >= Project.MSG_INFO) {
log(msg + e + msg2);
}
but this is subverting the log method that uses the verbosity
internally, and adding conditions to the construction of log messages is
not nice.
This does not create a log message - it just concatenates the given
arguments.
Would be something like toString(Object[]) better?
yes the name I chose was horrible (I'm severely lacking creativity today
obviously :)
You specify the args-param as "message fragments", so I would name the
paramter 'fragments' ;-)
public static String toString(final Object[] fragments)
...
that would make more sense yes
mmmh alternativly you could overload the Task.log() method...
Task.log(Object[] messageFragments, int loglevel)
Thanks for the comments
Kev
-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
