I also want to get my fine-toothed comb out (it works about the same as a microscope). If both Marcin and Bill are ready in the next couple of days I think we should let Marcin in first (we need to review his stuff too of course) since it sounds like his stuff will be less invasive. Then Bill can sync his trunk, then merge after that (pending any review changes)? From the sounds of things Marcin's patch simplifies class instantiation/subclassing logic quite a bit. It would be nice to commit these seperately so we can tell which one is causing any unforeseen regressions (not that I think either will have any issues :)).
Marcin, for largish patches you should follow Bill's lead here and create a branch so it is easier for all of us to see. Bill, It will be great to see how your code works in concurrent access tests. I have been looking at some of your commits as they come in, but you have made a lot changes so I have not looked as closely as I would have liked to. I will start looking in more earnest today (I cannot comment on your thread below yet). -Tom On 9/11/07, Charles Oliver Nutter <[EMAIL PROTECTED]> wrote: > Bill posted this comment to JRUBY-1339, and I figured it should be > re-posted here for discussion. > > From my perspective, I'm all for merging in the changes after a good > thorough review, but Bill and Marcin really need to sync up for that > merge since Marcin also has a fairly extensive patch that fixes a bunch > of allocate/initialize logic and the two patches are almost certain to > have conflicts. > > For the IRubyObject rework, I'm game to look at them and even to include > them in 1.1 if they seem pretty solid. If it helps bring us closer to > lightweights and directly invoking Java types, it will be so much for > the better as to outweigh much of the complexity. But when I say we need > to review it, we need to go over it with a microscope. > > Method renaming for consistency is great, but perhaps most likely to > cause merge conflicts. Pending review, I'm for including them. > > The ivar/cvar/constant rework sounds really promising, especially since > it's become apparent that ivar performance can be GREATLY improved. > > Bill's comment follows: > ----------------------- > > Yes – actually, I'd like to merge all the ivar/cvar/constant code > currently in bdortch/attrs back into trunk fairly soon, though it's not > quite ready (I hope to wrap it up tonight/tomorrow). It should be mostly > non-controversial (and definitely beneficial!), but there are a few > things that need some feedback/discussion (maybe not in this thread): > > * I've taken a preliminary stab at factoring/decomposing the > IRubyObject interface. I can omit/undo this for the merge back to trunk > if it needs more thought. The reason for doing this is to permit > (eventually) non-RubyObject/IRubyObject class/object implementations. > Not necessarily to support lightweights, either (though that would be a > goal) – there is a pretty good case to be made, for example, for not > defining the variableStore (was instanceVariables) and dataStruct fields > for most built-in types (storing vars externally for those types on the > rare occasion someone wants to, see experiment in bdortch/bnw), and > always instantiating a (final) variableStore for user-defined types. > Another interesting idea might be having RubyNumeric subclass > java.lang.Number, to eventually simplify method signatures that might > take raw/native (java.lang) or JRuby built-in values. > > * I've renamed some methods (and deprecated the old names) in an > effort to impart some consistency/clarity. The changes involving (the > overloading of) "instance variables" are really necessary – I've put a > lot of effort into untangling that knot, the current (trunk) > implementation being full of bugs, thread-safety violations, and so on. > The new code clearly distinguishes among ivars, cvars, constants and > "internal" variables (like those used when marshaling Ranges), and in > code I'm currently working on, actually treats them differently > (constant storage/lookup is optimized). Some of the other changes > (searchConstant in place of getConstant) are primarily for clarity. I've > retained, but deprecated, the old names. > > As I said, I should have this stuff wrapped up in the next day or so. > There are a couple of pieces not yet in SVN, notably a > ConcurrentModuleVariableStore (in progress) that includes the constant > optimization I mentioned, and also a concurrent implementation of > RubySymbol.SymbolTable that shows a 15% improvement in bench_symbol > before I've even hooked up the "fast" lookup method. The performance > gains of all the "concurrent" code should be even more pronounced in > multi-threaded benchmarks (I'm working on some). > > -Bill > > --------------------------------------------------------------------- > To unsubscribe from this list please visit: > > http://xircles.codehaus.org/manage_email > > -- Blog: http://www.bloglines.com/blog/ThomasEEnebo Email: [EMAIL PROTECTED] , [EMAIL PROTECTED] --------------------------------------------------------------------- To unsubscribe from this list please visit: http://xircles.codehaus.org/manage_email
