Hello all,
since my merge of the less-reflection branch with trunk[1], several of you have been bitten by the new limitation in the number of functions that can be contained in a FASL. The reflection-less scheme requires creating a loader class because it needs to spell out in the bytecode all the class names corresponding to compiled Lisp functions in the FASL. Functions are numbered from 1 onwards and a method is created that is a huge switch (actually a chain of if's) that returns the right function given its index:
if(index >= 1 && index < 1024) { if(index == 1) return new function_1(); if(index == 2) return new function_2(); ... } else if(index >= 1024 ...
In the presence of many functions, this method grows very large, eventually surpassing the JVM's 64k method size limit.
A couple of days ago Erik and I discussed a bit about the issue and how to solve it. One option is to implement method splitting for the big FASL loader method. Method splitting in general is hard (you'd need some form of CPS conversion) but in this case it's quite easy. The current scheme, though, has drawbacks: it's more complicated than it used to be, and it's not that beneficial performance-wise - from a profiling I did a while ago, its performance gains amounted to ~1% of our loading time, which is completely dominated by I/O and UTF-8 decoding. So, there's another option: go back to reflection-based loading. Using reflection has the benefit of being simpler. On certain platforms (e.g. mobile JVMs or similar, like Dalvik), reflection has a higher cost, but we currently don't run satisfactorily on those platforms anyway and we'd require multiple changes to do so. After my discussion with Erik, a further refinement of this second option came to my mind. As part of the no-reflection effort, the compiler was changed to stop emitting calls to loadCompiledFunction(className) in favor of generating `new className()` bytecode for local functions, in accordance with the goal of avoiding reflection as much as possible. A special classloader is used to both resolve those local function references, and to load the FASL loader class (and thus transitively all top-level functions). I propose keeping the compilation of local function references as it is, and thus the special classloader; and use it directly (via reflection) to load top-level functions, instead of generating one big loader method. This would result in slightly less reflection, but more importantly in a clearer function loading scheme, and less development effort compared to returning completely to the reflection-based scheme also for local functions.
What do you think? If it's not clear, both Erik and I sponsor the solution of returning to a reflection-based approach, but we'd like to hear everyone's opinions.
Bye, Alessio
On 12 January 2011 16:22, Alessio Stalla alessiostalla@gmail.com wrote:
What do you think? If it's not clear, both Erik and I sponsor the solution of returning to a reflection-based approach, but we'd like to hear everyone's opinions.
I think it would perhaps be a good idea to revert to reflection-loading, put that revert into 0.24 before we release, and patch this idea in for 0.25. That is assuming that the revert is easy to do.
On Wed, Jan 12, 2011 at 3:35 PM, Ville Voutilainen ville.voutilainen@gmail.com wrote:
On 12 January 2011 16:22, Alessio Stalla alessiostalla@gmail.com wrote:
What do you think? If it's not clear, both Erik and I sponsor the solution of returning to a reflection-based approach, but we'd like to hear everyone's opinions.
I think it would perhaps be a good idea to revert to reflection-loading, put that revert into 0.24 before we release, and patch this idea in for 0.25. That is assuming that the revert is easy to do.
Reverting is hard, as the merge is 8 months old, stuff has changed since, and not everything should be reverted. It'll be mostly a manual process. So the less stuff is reverted, the better. That's part of the reason for my last proposal.
A.
On 12 January 2011 17:10, Alessio Stalla alessiostalla@gmail.com wrote:
Reverting is hard, as the merge is 8 months old, stuff has changed since, and not everything should be reverted. It'll be mostly a manual process. So the less stuff is reverted, the better. That's part of the reason for my last proposal.
If the last proposal is easier, I'm all for it. The general goal is to try and fix the huge-function problem before we release 0.24.
On Wed, Jan 12, 2011 at 4:33 PM, Ville Voutilainen ville.voutilainen@gmail.com wrote:
On 12 January 2011 17:10, Alessio Stalla alessiostalla@gmail.com wrote:
Reverting is hard, as the merge is 8 months old, stuff has changed since, and not everything should be reverted. It'll be mostly a manual process. So the less stuff is reverted, the better. That's part of the reason for my last proposal.
If the last proposal is easier, I'm all for it. The general goal is to try and fix the huge-function problem before we release 0.24.
I implemented the proposed change in revision 13135 [1]. I saw no noticeable increase in compilation or startup time. I was able to compile FASLs with up to 20000 functions (with 25000 it crashed with a OOM exception). I also ran the test suite, and I had to fix an unrelated bug in Package.java to make it run to completion. I have to test if disassemble still works, and then I'd say that the change can go into 0.24 for me.
[1] http://trac.common-lisp.net/armedbear/changeset/13135
Alessio
armedbear-devel@common-lisp.net