On 10/6/07, MenTaLguY <[EMAIL PROTECTED]> wrote: > > On Sat, 2007-10-06 at 16:04 -0700, Bill Dortch wrote: > > - Implementing the tables directly rather than as a component > > provided a significant performance benefit -- somewhat to my surprise. > > I did originally take the latter approach. The code is in > > bdortch/attrs/src/org/jruby/runtime/component, in VariableStore > > (abstract base class) and ConcurrentObjectVariableStore, which may yet > > find some use (in a somewhat modified form) once we refactor > > IRubyObject/RubyObject. > > Hmm. What about performance as a standalone class? The addition of an > abstract base class can potentially have a performance impact by itself, > since it complicates method dispatching.
As I recall now, part of the problem was that the RubyObject field for the variableStore also needed to be volatile, since we don't want to create one at the outset (yet another reason to refactor the RubyObject hierarchy -- for user-defined classes, I would always create a variable table/store, while for built-ins I'd store ivars externally [see experiment in bdortch/bnw]). So this added another volatile read. BTW, currently (in trunk), the instanceVariables map is accessed unsafely (double-checked locking idiom). > However, I don't think it's unreasonable to implement these directly > > for performance (see RubyArray and RubyHash, for instance). > > I guess so ... I'm just a little worried about the code getting > copy-and-pasted different places (e.g. for constant tables or something) > this early in its life-cycle. Um, did you take a look at the code in RubyModule as well? Constants are, in fact, stored separately (different entry type with final value), so much of the code was copied/pasted. (Also, the variableTableStore/Remove/Sync methods are reimplemented to take advantage of the ReentrantLock defined for classes/modules.) If you could, please take a look (if you haven't already) and let me know if you see any problems (other than the readValueUnderLock issue). Thanks. -Bill
