[armedbear-devel] Consolidating copy/paste code
Yesterday Ville and I dedicated some time to the consolidation of copy/paste code and unnecessary virtual methods. There's quite a bit of copy/paste code in ABCL. A good example are LispReader, FaslReader and Stream.java. FaslReader had a number of functions which were *exactly* equal to functions in LispReader. A number of functions contained the same logic, but used different readtables. The same applies to functions in Stream.java where functions were declared twice - once with a faslRead* prefix and once with a read* prefix. The solution is - in this case, of course - to abstract the code from the readtable to be used. In other places the reasons for copy/pasting will be different. If you're lurking at this list, thinking of contributing but you don't know how: this is a much appreciated way to help out! We try to reduce the number of (copy/pasted) lines of code in ABCL; not at the expense of functionality, but as a way to ensure consistent maintenance in the future. Still no idea where to start? I left some remaining work in LispReader.java and FaslReader.java where there is more copy/pasted code. My idea is that the common bits get moved to Stream.java, using the ReadtableAccessor class to abstract from the readtables themselves. Looking forward to your contributions! Bye, Erik.
OK, Sounds like a nice afternoon project. Here's a patch that does what I think you're asking in FaslReader and LispReader. But I ran into some trouble testing the changes. Following the instructions at http://common-lisp.net/project/armedbear/contributing.shtml I end up with an OutOfMemoryError when running both test.ansi.interpreted and test.ansi.compiled Is there an enviroment variable that needs to be set to get these to run? I can make it work by putting <jvmarg value="-Xmx500M"> lines in build.xml, which I've attached as a second patch. When I ran the tests after I made the change, I got one additional error when running test.ansi.interpreted: PRINT.BACKQUOTE.RANDOM.14. I've also attached the error. I can't figure it out. I eventually decided that a 'random' test might give a different result if I run it again ... so I ran it again (and again and again) ... and the error did not happen again. So I now suspect that my patch does not introduce any new errors, but that PRINT.BACKQUOTE.RANDOM.14 fails randomly from time to time. Cheers, -david k. On Sun, Apr 11, 2010 at 1:26 AM, Erik Huelsmann <ehuels@gmail.com> wrote:
Yesterday Ville and I dedicated some time to the consolidation of copy/paste code and unnecessary virtual methods.
There's quite a bit of copy/paste code in ABCL. A good example are LispReader, FaslReader and Stream.java. FaslReader had a number of functions which were *exactly* equal to functions in LispReader. A number of functions contained the same logic, but used different readtables. The same applies to functions in Stream.java where functions were declared twice - once with a faslRead* prefix and once with a read* prefix.
The solution is - in this case, of course - to abstract the code from the readtable to be used. In other places the reasons for copy/pasting will be different.
If you're lurking at this list, thinking of contributing but you don't know how: this is a much appreciated way to help out!
We try to reduce the number of (copy/pasted) lines of code in ABCL; not at the expense of functionality, but as a way to ensure consistent maintenance in the future.
Still no idea where to start? I left some remaining work in LispReader.java and FaslReader.java where there is more copy/pasted code. My idea is that the common bits get moved to Stream.java, using the ReadtableAccessor class to abstract from the readtables themselves.
Looking forward to your contributions!
Bye,
Erik.
_______________________________________________ armedbear-devel mailing list armedbear-devel@common-lisp.net http://common-lisp.net/cgi-bin/mailman/listinfo/armedbear-devel
On 4/12/10 2:59 AM, David Kirkman wrote:
OK, Sounds like a nice afternoon project. Here's a patch that does what I think you're asking in FaslReader and LispReader.
But I ran into some trouble testing the changes. Following the instructions at http://common-lisp.net/project/armedbear/contributing.shtml I end up with an OutOfMemoryError when running both test.ansi.interpreted and test.ansi.compiled
For most platforms we have tested ABCL on, it is expected to not be able to complete the ANSI tests without increasing the default stack size of the JVM. We haven't made this the default in the build process as there is large variability across platforms that makes a standard setting cause errors on some platforms immediately but prohibit the "right" amount of memory on others. And unfortunately we can't set memory programatically at runtime by querying what the platform limits would be. The only approach that makes sense For the ANSI tests, I have found through experience on OSX, WinXP, and OpenSolaris with Java6, that one needs at least 512MB of stack. Through experimentation it seems that the tests should complete in around 300-500 seconds on contemporary x86 machines. If your tests take substantially longer, the JVM is probably starved for heap, and would complete the tests faster by playing with memory limits.
Is there an enviroment variable that needs to be set to get these to run? I can make it work by putting<jvmarg value="-Xmx500M"> lines in build.xml, which I've attached as a second patch.
For the Ant-based build, one sets theJava property 'java.options' in the 'abcl.properties' file to set Java options in the wrapper scripts. For example I use the following line for OSX as my standard configuration: java.options=-Xms1g -Xmx4g One could also pass this as something like ''-Djava.options="-Xms1g -Xmx4g"'' to the Ant process (not sure of the exact syntax here). This should be documented better (my fault!)
When I ran the tests after I made the change, I got one additional error when running test.ansi.interpreted: PRINT.BACKQUOTE.RANDOM.14. I've also attached the error. I can't figure it out. I eventually decided that a 'random' test might give a different result if I run it again ... so I ran it again (and again and again) ... and the error did not happen again. So I now suspect that my patch does not introduce any new errors, but that PRINT.BACKQUOTE.RANDOM.14 fails randomly from time to time.
I have the same experience of random test failure here. The test does indeed randomly generate forms to test backquote, but doesn't record what input was used. Another task here would be to re-write the test to emit the tested form, run it enough times to get it to fail, and analyze the results to try to determine what aspect of it is failing. -- "A screaming comes across the sky. It has happened before, but there is nothing to compare to it now."
On 4/12/10 2:27 PM, Mark Evenson wrote: […]
And unfortunately we can't set memory programatically at runtime by querying what the platform limits would be. The only approach that makes sense
…would be to have a database of reasonable values that we somehow compile into UNIX shell scripts and DOS batch files to do the right thing at runtime. Or maybe, have an initial JVM launch a secondary JVM with the correct settings, but I don't know if this is possible. These ideas have always seemed to be too much trouble to me, and would anyhow require that we collect the database of values in the first place. There was some hope that Java7 would address this sort of problem, as it effects almost all serious cross-platform use of the JVM which has been solved piece-meal by each individual application deployment. Anyways, that has been the extent of my thinking on the problem (and I think summarizes the various discussions on #abcl). Other possible solutions would be welcome. […] -- "A screaming comes across the sky. It has happened before, but there is nothing to compare to it now."
On Mon, Apr 12, 2010 at 5:27 AM, Mark Evenson <evenson@panix.com> wrote:
For the Ant-based build, one sets theJava property 'java.options' in the 'abcl.properties' file to set Java options in the wrapper scripts. For example I use the following line for OSX as my standard configuration:
java.options=-Xms1g -Xmx4g
One could also pass this as something like ''-Djava.options="-Xms1g -Xmx4g"'' to the Ant process (not sure of the exact syntax here).
I originally tried setting java.options in abcl.properties, but no setting I tried had any effect (but the maximum I tried was -Xms1g -Xmx1g -- I'm running on an old laptop without very much memory). I also played with setting JAVA_OPTIONS and ANT_OPTIONS. (I also just retried it, and it still doesn't work!) I might just be having a problem because of this old machine (mac os 10.4 (power pc) ant version 1.7.1, and java version 1.5.0_19
This should be documented better (my fault!)
It seemed pretty clear after I got the first out of memory error that java.options should be meddled with, so I think the documentation is working fine! It looks like there is just a bug in the way it interacts with my old system.
Hi David, On Mon, Apr 12, 2010 at 2:59 AM, David Kirkman <dkirkman@ucsd.edu> wrote:
OK, Sounds like a nice afternoon project. Here's a patch that does what I think you're asking in FaslReader and LispReader.
Exactly what I was talking about.
But I ran into some trouble testing the changes. Following the instructions at http://common-lisp.net/project/armedbear/contributing.shtml I end up with an OutOfMemoryError when running both test.ansi.interpreted and test.ansi.compiled
That's probably where 30 minutes turned into an afternoon after all? :-) I applied your patch, checked it for tabs (which I couldn't find: perfect!), ran the tests and committed it as r12604. Thanks again for your contribution! Do you feel like contributing more, but you don't have any idea where to start? You might want to have a look at ticket 88; it's a change to provide ABCL with primitives for SLIME to use to find the right source code. There's a comment in the ticket explaining my direction of thoughts. If that's not enough, we can discuss on the list, here. Although this is probably more like 2 or 3 afternoons since you're very new in our code base. Thanks again for your contribution! Bye, Erik.
On Wed, Apr 14, 2010 at 1:33 PM, Erik Huelsmann <ehuels@gmail.com> wrote:
But I ran into some trouble testing the changes. Following the instructions at http://common-lisp.net/project/armedbear/contributing.shtml I end up with an OutOfMemoryError when running both test.ansi.interpreted and test.ansi.compiled
That's probably where 30 minutes turned into an afternoon after all? :-)
And that's an uncannily accurate description of Sunday afternoon!
Do you feel like contributing more, but you don't have any idea where to start? You might want to have a look at ticket 88; it's a change to provide ABCL with primitives for SLIME to use to find the right source code. There's a comment in the ticket explaining my direction of thoughts. If that's not enough, we can discuss on the list, here.
Although this is probably more like 2 or 3 afternoons since you're very new in our code base.
OK. I'll take a look at it this weekend. It sounds like you're suggesting to modify the compiler to emit CompiledPrimitive instead of Primitive, where CompiledPrimitive is just an empty sublcass of Primitive?
Thanks again for your contribution!
No trouble. abcl just allowed me to integrate a large amount of old code that I was rather fond of with an equally large amount of new code, so I'm rather grateful. -david k.
Hi David,
Do you feel like contributing more, but you don't have any idea where to start? You might want to have a look at ticket 88; it's a change to provide ABCL with primitives for SLIME to use to find the right source code. There's a comment in the ticket explaining my direction of thoughts. If that's not enough, we can discuss on the list, here.
Although this is probably more like 2 or 3 afternoons since you're very new in our code base.
OK. I'll take a look at it this weekend. It sounds like you're suggesting to modify the compiler to emit CompiledPrimitive instead of Primitive, where CompiledPrimitive is just an empty sublcass of Primitive?
Yes. Additionally, it would be a good idea to define a Primitive (in Primitives.java) which takes exactly one argument and returns T if that argument is instanceof CompiledPrimitive or CompiledClosure; or NIL otherwise.
Thanks again for your contribution!
No trouble. abcl just allowed me to integrate a large amount of old code that I was rather fond of with an equally large amount of new code, so I'm rather grateful.
If you need help or pointers, please do ask! We'll gladly help you find your way around. Bye, Erik.
On Thu, Apr 15, 2010 at 2:21 PM, Erik Huelsmann <ehuels@gmail.com> wrote:
Hi David,
Do you feel like contributing more, but you don't have any idea where to start? You might want to have a look at ticket 88; it's a change to provide ABCL with primitives for SLIME to use to find the right source code. There's a comment in the ticket explaining my direction of thoughts. If that's not enough, we can discuss on the list, here.
Although this is probably more like 2 or 3 afternoons since you're very new in our code base.
OK. I'll take a look at it this weekend. It sounds like you're suggesting to modify the compiler to emit CompiledPrimitive instead of Primitive, where CompiledPrimitive is just an empty sublcass of Primitive?
Yes. Additionally, it would be a good idea to define a Primitive (in Primitives.java) which takes exactly one argument and returns T if that argument is instanceof CompiledPrimitive or CompiledClosure; or NIL otherwise.
I had implemented ticket #88 in my Lisps SVN a month back.. (since it was based on ABCL the change should be easy to see) What I do is extend JavaPrimitive instead of Primitive. created as: https://code.google.com/r/logicmoo-invoke-interface/source/browse/src/com/cyc/tool/subl/jrtl/nativeCode/commonLisp/JavaPrimitive.java?spec=svn4bce7566fdb7cec203b098526ff83c28362f0256&r=4bce7566fdb7cec203b098526ff83c28362f0256 Whenever a primitive is defined from .java source I force JavaPrimitive instead of Primitive then for JavaPrimitive Here is the scope of the change: https://code.google.com/r/logicmoo-invoke-interface/source/detail?r=e95a8df3af8704970bdfd5934e6a54d3834128ac&path=/src/com/cyc/tool/subl/jrtl/nativeCode/commonLisp/StringFunctions.java Jope you find this helpfull.
Thanks again for your contribution!
No trouble. abcl just allowed me to integrate a large amount of old code that I was rather fond of with an equally large amount of new code, so I'm rather grateful.
If you need help or pointers, please do ask! We'll gladly help you find your way around.
Bye,
Erik.
_______________________________________________ armedbear-devel mailing list armedbear-devel@common-lisp.net http://common-lisp.net/cgi-bin/mailman/listinfo/armedbear-devel
Erik Huelsmann <ehuels@gmail.com> writes:
Do you feel like contributing more, but you don't have any idea where to start? You might want to have a look at ticket 88; it's a change to provide ABCL with primitives for SLIME to use to find the right source code. There's a comment in the ticket explaining my direction of thoughts. If that's not enough, we can discuss on the list, here.
Here's what to do afterwards (perhaps my Mark): * change the build process to create a TAGS file if not already done. * set up logical pathname translations to easily find the source directory, and hence the TAGS file. -T.
On 4/15/10 1:13 PM, Tobias C. Rittweiler wrote:
Erik Huelsmann<ehuels@gmail.com> writes:
Do you feel like contributing more, but you don't have any idea where to start? You might want to have a look at ticket 88; it's a change to provide ABCL with primitives for SLIME to use to find the right source code. There's a comment in the ticket explaining my direction of thoughts. If that's not enough, we can discuss on the list, here.
Here's what to do afterwards (perhaps my Mark):
* change the build process to create a TAGS file if not already done.
The target 'TAGS' already creates a TAGS file, so this is done.
* set up logical pathname translations to easily find the source directory, and hence the TAGS file.
There is a logical pathname translation for the Java src as SYS:JAVA pointing to the root of Java source tree, and SYS:SRC pointing to the Lisp source. See 'system.lisp' as generated by the build system. Tobias made some detailed notes for suggestions concerning the modernization of the source location system. Is it ok if I add these directly to ticket #88 Tobias? -- "A screaming comes across the sky. It has happened before, but there is nothing to compare to it now."
Mark Evenson <evenson@panix.com> writes:
On 4/15/10 1:13 PM, Tobias C. Rittweiler wrote:
Erik Huelsmann<ehuels@gmail.com> writes:
Do you feel like contributing more, but you don't have any idea where to start? You might want to have a look at ticket 88; it's a change to provide ABCL with primitives for SLIME to use to find the right source code. There's a comment in the ticket explaining my direction of thoughts. If that's not enough, we can discuss on the list, here.
Here's what to do afterwards (perhaps my Mark):
* change the build process to create a TAGS file if not already done.
The target 'TAGS' already creates a TAGS file, so this is done.
Does the main target include the TAGS target?
* set up logical pathname translations to easily find the source directory, and hence the TAGS file.
There is a logical pathname translation for the Java src as SYS:JAVA pointing to the root of Java source tree, and SYS:SRC pointing to the Lisp source. See 'system.lisp' as generated by the build system.
Tobias made some detailed notes for suggestions concerning the modernization of the source location system. Is it ok if I add these directly to ticket #88 Tobias?
Yes, sure.
On Apr 15, 2010, at 1:41 PM, Tobias C. Rittweiler wrote: […]
* change the build process to create a TAGS file if not already done.
The target 'TAGS' already creates a TAGS file, so this is done.
Does the main target include the TAGS target?
No, it currently doesn't as what to do if there isn't an 'etags' executable in the current path isn't well defined (i.e. under win32). Getting Ant to do this correctly probably involves conditional execution (perhaps with the <available> task?) This would take a little bit of fiddling to get right, with the usual testing across our supported platforms. If someone wants to take a stab at this, I will help test and sponsor a commit, but right now my stack is a little too full to take on additional tasks.
Tobias made some detailed notes for suggestions concerning the modernization of the source location system. Is it ok if I add these directly to ticket #88 Tobias?
Yes, sure.
I added the [gist of Tobias' analysis in the Wiki][1]. [1]: http://trac.common-lisp.net/armedbear/wiki/SourceLocation -- "A screaming comes across the sky. It has happened before, but there is nothing to compare to it now."
On Thu, Apr 15, 2010 at 9:20 AM, Mark Evenson <evenson@panix.com> wrote:
I added the [gist of Tobias' analysis in the Wiki][1].
[1]: http://trac.common-lisp.net/armedbear/wiki/SourceLocation
Regarding the comment:
SWANK-COMPILE-STRING must be implemented on top of COMPILE-FILE, the current implementation is wrong.
Note that slime may assume that a running lisp has access to a writeable file system, which may not be the case. So this is probably too strong a constraint. -Alan
On Thu, Apr 15, 2010 at 3:29 PM, Alan Ruttenberg <alanruttenberg@gmail.com> wrote:
On Thu, Apr 15, 2010 at 9:20 AM, Mark Evenson <evenson@panix.com> wrote:
I added the [gist of Tobias' analysis in the Wiki][1].
[1]: http://trac.common-lisp.net/armedbear/wiki/SourceLocation
Regarding the comment:
SWANK-COMPILE-STRING must be implemented on top of COMPILE-FILE, the current implementation is wrong.
Note that slime may assume that a running lisp has access to a writeable file system, which may not be the case. So this is probably too strong a constraint.
In general I think that ABCL would benefit from a COMPILE-STRING function which works exactly like COMPILE-FILE but performs all I/O in memory. Given Mark's Pathname URL support, maybe we could have a "special" url scheme (say, abcl-memory) to represent in-memory "files"? This would perhaps make such a function easier to implement. Just my .02, Alessio
Alan Ruttenberg <alanruttenberg@gmail.com> writes:
On Thu, Apr 15, 2010 at 9:20 AM, Mark Evenson <evenson@panix.com> wrote:
I added the [gist of Tobias' analysis in the Wiki][1].
[1]: http://trac.common-lisp.net/armedbear/wiki/SourceLocation
Regarding the comment:
SWANK-COMPILE-STRING must be implemented on top of COMPILE-FILE, the current implementation is wrong.
Note that slime may assume that a running lisp has access to a writeable file system, which may not be the case. So this is probably too strong a constraint.
It's not about COMPILE-FILE per se: whatever is used should have the same semantics as file compilation, that's the point. Some implementations provide a COMPILE-FROM-STREAM or similiar where a string-stream can be feed to, for instance. Going over a temp file just happens to be the quickest road to success. -T.
On Thu, Apr 15, 2010 at 9:46 AM, Tobias C. Rittweiler <tcr@freebits.de> wrote:
Alan Ruttenberg <alanruttenberg@gmail.com> writes:
On Thu, Apr 15, 2010 at 9:20 AM, Mark Evenson <evenson@panix.com> wrote:
I added the [gist of Tobias' analysis in the Wiki][1].
[1]: http://trac.common-lisp.net/armedbear/wiki/SourceLocation
Regarding the comment:
SWANK-COMPILE-STRING must be implemented on top of COMPILE-FILE, the current implementation is wrong.
Note that slime may assume that a running lisp has access to a writeable file system, which may not be the case. So this is probably too strong a constraint.
It's not about COMPILE-FILE per se: whatever is used should have the same semantics as file compilation, that's the point.
It can't have the same semantics as file compilation if it doesn't create a file. I get the idea, of course, but it would be good to clarify the expectations by giving the explicit requirements. Best, Alan
Some implementations provide a COMPILE-FROM-STREAM or similiar where a string-stream can be feed to, for instance. Going over a temp file just happens to be the quickest road to success.
-T.
_______________________________________________ armedbear-devel mailing list armedbear-devel@common-lisp.net http://common-lisp.net/cgi-bin/mailman/listinfo/armedbear-devel
Alan Ruttenberg <alanruttenberg@gmail.com> writes:
It can't have the same semantics as file compilation if it doesn't create a file. I get the idea, of course, but it would be good to clarify the expectations by giving the explicit requirements.
CLHS 3.2.3, "File Compilation" -T.
participants (7)
-
Alan Ruttenberg -
Alessio Stalla -
David Kirkman -
dmiles＠users.sourceforge.net -
Erik Huelsmann -
Mark Evenson -
Tobias C. Rittweiler