> -----Original Message----- > From: Geir Magnusson Jr. [mailto:[EMAIL PROTECTED] > Sent: Sunday, September 24, 2006 2:37 PM > To: harmony-dev@incubator.apache.org > Subject: Re: [classlib][vmi] VMI classes for Thread/Object manipulation > for java.util.concurrent > > > On Sep 24, 2006, at 3:29 PM, Nathan Beyer wrote: > > > I made some updates to Threads and Objects based on the feedback in > > this > > thread. I've also implemented Unsafe in terms of these two classes, > > just to > > validate the code path. > > > > Some comments based on observations in the code and some items in this > > thread. > > > > * ObjectAccessor in misc currently can't fulfill the Unsafe > > contract; it's > > missing volatile get/set. This is also missing for array element > > volatile > > get/set and there's no mapping from Unsafe's relative/scaled access > > for > > array elements. > > > > * I don't think there is any value in separating the atomic compare > > and swap > > method into a concurrency-kernel; there are only three method > > (int/long/Object), the methods use the same fieldOffset/Field ref > > mechanism > > that the Objects/ObjectAccessor does, so this would have to be > > duplicated in > > the interface or a consumer would have to use Objects/ > > ObjectAccessor to work > > with an Atomics class, which creates a loose coupling anyway. > > > > * I think we should utilize the Accessor classes in 'misc' (in > > place of > > Objects), but I think these all need to be refactored into a kernel > > or VM > > module. I really don't like the 'misc' naming, as it make it seem > > like it's > > just some extra crap that doesn't fit in any category, which seems > > very > > inappropriate as these are core VM services and utilities used by many > > modules. > > This isn't our fault that it's named that way. I think that all it > does is make suncompat a required piece, something I have no > objection to :)
Are you referring to "sun.misc.Unsafe"? If so, my statement was confusing; I was referring to the "org.apache.harmony.misc" stuff - I don't like that Harmony has a 'misc' sub package and module. I know we can't do anything the suncompat stuff. Sorry for confusing you. -Nathan > > > > > My suggestion would be to move the accessor parts of 'misc' into > > kernel, put > > them in the "vm" package and add some "getInstance" methods for > > security. > > > > -Nathan > > > > > >> -----Original Message----- > >> From: Andrey Chernyshev [mailto:[EMAIL PROTECTED] > >> Sent: Sunday, September 24, 2006 6:44 AM > >> To: harmony-dev@incubator.apache.org > >> Subject: Re: [classlib][vmi] VMI classes for Thread/Object > >> manipulation > >> for java.util.concurrent > >> > >> On 9/22/06, Tim Ellison <[EMAIL PROTECTED]> wrote: > >>> Andrey Chernyshev wrote: > >>>> On 9/20/06, Tim Ellison <[EMAIL PROTECTED]> wrote: > >>>>> Andrey Chernyshev wrote: > >>>>>> Thanks Nathan! The Threads interface looks fine. Still, may be it > >>>>>> would be nice if two different methods are allocated for > >>>>>> parkNanos > >> and > >>>>>> parkUntil - passing the extra boolean parameter seems like an > >>>>>> overhead, though very little. > >>>>> > >>>>> I agree, just create another method rather than passing in a > >>>>> boolean > >>>>> flag. > >>>>> > >>>>> How are you going to avoid apps calling these public methods? > >>>>> We can > >> do > >>>>> a security/calling stack check on each method send, but it may be > >>>>> preferable to make Threads a singleton and check in a > >>>>> getSingleton() > >>>>> call. > >>>> > >>>> Yes, accessor classes were also designed as sigletones those > >>>> instances > >>>> can only be obtained through the AccessorFactory which handles the > >>>> security checks. > >>>> I wonder if it may make sense to have a single factory for accessor > >>>> classes and Threads. > >>> > >>> Just let each type handle the security check. > >> > >> Good suggestion. So it sounds like the ObjectAccessor, ArrayAccessor > >> and other accessors should be created with the static > >> XXXAccessor.getInstance() calls (instead of Factory.getXXXAccessor > >> calls)? > >> Yes, it would help to split accessors, though probably at the > >> price of > >> the duplication of the security checking code. > >> Right now it is in the AccessorFactory.checkPermissions()), but it > >> will have to be replicated in Atomics (or whatever) from the > >> concurrent-kernel if we want to avoid dependencies between > >> concurrent-kernel and o.a.h.accessors package. > >> > >> > >>> > >>> <snip> > >>> > >>>>> Do these need to be rearranged? Why can't we write the > >>>>> suncompat's > >>>>> Unsafe equivalents in terms of these accessors? > >>>> > >>>> I wouldn't wish we a have multiple classes which are doing the same > >>>> things, even if some of them are delegating work to others (e.g. > >>>> Objects is written on top of accessors or accessors are > >>>> rewritten on > >>>> top of Objects) - this seems to me just like extra layers/function > >>>> calls. > >>> > >>> Agreed. I suggest that we have separate types for VM-specific > >>> vs. JNI > >>> based accessors so we can have a clean kernel code and common code > >> split. > >>> > >>> As written elsewhere, the only delegation/adapter code will be from > >>> suncompat Unsafe to o.a.harmony types. > >>> > >>>>>> plus the o.a.util.concurrent.Atomics [5] from the DRLVM. > >>>>> > >>>>> Yep, these need to be moved into the kernel for all VMs to > >>>>> implement. > >>>>> We can define them in (a new) concurrent-kernel unless there is > >>>>> consensus that they would be more generally useful, i.e. misc- > >>>>> kernel > >> or > >>>>> luni-kernel. > >>>> > >>>> If all VM's are supposed to be 1.5+ compliant anyways, why not just > >>>> adding to the luni-kernel... > >>> > >>> Because we want to keep the modularity of concurrency utils separate > >>> from LUNI. If there is no need for atomic/volatile operations > >>> outside > >>> the concurrency module, then we should put them into a concurrent- > >> kernel. > >>> > >> > >> There is a set of informational methods one would need to access > >> objects and arrays, regardless of whether the access is going to be > >> ordinary, volatile or atomic. They are sort of like (I'm taking the > >> signatures from the accessor package): > >> > >> long getFieldID(Field f); > >> long getArrayBaseOffset(Class arrayClass) > >> int getArrayElementSize(Class arrayClass) > >> > >> (Unsafe also has the similar method triple called objectFieldOffset > >> (), > >> arrayBaseOffset(), arrayIndexScale()). If we are doing a split > >> between the different types of access, then the questions are: > >> > >> - Should the implementations of the above 3 methods be the part of > >> the > >> accessor or the kernel classes set?, > >> - Should they be duplicated in both accessors and kernel? > >> - If we keep them in the accessors package where they currently are, > >> would it be OK for concurrent-kernel to have the dependencies on the > >> accessors package? > >> > >> > >>> <snip> > >>> > >>>>> Andrey: did you check that everything in Objects is covered by > >> existing > >>>>> accessor/atomics? > >>>> > >>>> Yes, the Objects is a subset of the existing accessors + Atomics > >>>> combination. > >>> > >>> Then how about we delete 'Objects' and implement those parts of > >>> Unsafe > >>> in terms of existing ObjectAccessor methods? > >> > >> I think it would be possible, the only problem that I can see is a > >> slight difference between the types which are used in the accessors > >> and in the Unsafe: > >> - Unsafe works in terms of offsets and is using same offsets for > >> accessing objects and arrays. For ex, once you get an offset to a > >> specific element of an array, you can then use that offset for > >> calling > >> putField() method for array just like it was an ordinary object. > >> - in the accessor package, objects and arrays are separate - objects > >> are accessed with fieldID while arrays are accessed with indexes. > >> This > >> was done because the underlying JNI implementations of those accesses > >> will be different. > >> > >> The missing piece in the current combination of the Atomics and > >> accessors is a volatile access to array. There could be two > >> approaches > >> to deal with that: > >> 1) Go with the current design of the accessors and add > >> get/setXXXVolatile() methods for long/integer/boolean arrays to the > >> Atomics; > >> 2) Slightly redesign the accessors package and make it more > >> Unsafe-like - e.g. use the same offsets for objects and arrays. It > >> may > >> lead, however, to some performance degradation - for the set/getXXX > >> methods of the ObjectAccessor, we would have to check if the > >> Object is > >> array and use the different implementation in JNI code then. > >> > >>> > >>> <snip> > >>> > >>>>> Do accessors need to be in kernel? They are implemented solely in > >> terms > >>>>> of JNI - right? > >>>> > >>>> Right. It is clear that now we have a set of API's that provide > >>>> different type of access to objects (e.g. simple, atomic or > >>>> volatile), > >>>> arrays and threads. I can imagine the following types of it's > >>>> classification: > >>>> - VM specific or not (or, can be implemented with JNI or not) > >>> > >>> yep, this is our kernel vs. common code distinction. > >>> > >>>> - Package where they are needed (e.g. j.u.c, awt/swing, io or > >> whatever) > >>> > >>> yep, we need to judge where the best place is for each type. > >>> Since they > >>> are o.a.harmony types the risk of putting them too 'low' (e.g. > >>> luni) is > >>> that they bloat the module where they are not actually used; and the > >>> risk of putting them too 'high' (e.g. swing) is that we create > >>> dependencies 'up' the semantic stack. > >>> > >>> In this instance I think we only need to decide between whether > >>> accessors go into luni, concurrent, and misc (maybe misc gets rolled > >>> into those two?). > >> > >> Probably the most natural place for the accessors would be somewhere > >> at luni - we may consider them as covering some gaps in the lang API > >> which are not allowing the classlib developers to access objects and > >> arrays easily. > >> > >>> > >>>> - Objects they are accessing (e.g. object, array, thread) > >>> > >>> Do you think we need this distinction in the type hierarchy? All > >>> accessors work on fundamental types, right? > >> > >> I think the distinction between Arrays and Objects in the accessor > >> package is caused by the difference how the JNI spec treats the > >> objects and arrays. For arrays, JNI allows "raw" access to the memory > >> region occupied by the array in Java heap. Hence the thing you'd need > >> to know is an array type (array element size) and index within the > >> array. For objects, JNI doesn't expose anything except the abstract > >> fieldID. Those ID's may not necessarily represent the offsets within > >> the memory of object. > >> As I wrote earlier, we may avoid such distinction at some lost of the > >> performance and adding some pointer arithmetic which would translate > >> offsets in the arrays back to indexes which are used as the > >> parameters > >> for JNI calls. It would be like: > >> index = <offset provided by user code> / <array element size>, > >> while the user would do in the code: > >> offset = <array element size> * <arrayIndexScale> + <arrayBaseOffset> > >> > >> In other words, if we use JNI to implement access to the arrays and > >> keep Unsafe-like interface, we'll be doing extra computations > >> converting each time indexes to offsets in classlib code and then > >> converting offsets back to the indexes in the accessor's > >> implementation code. > >> > >> > >>> > >>>> We may pick one or another type of classification and split the API > >>>> into different pacakges/jars in accordance with it. > >>>> > >>>> On the other hand, obviously all of these classes would have the > >>>> same > >>>> mechanism for getting their instances, and most likely share the > >>>> way > >>>> how fieldID's (or offsets) are obtained. In other words, it may be > >>>> unnatural to try to split them. This is why I proposed to keep > >>>> them in > >>>> a single place. > >>> > >>> If we can think of a usecase for volatile/atomic operations outside > >>> concurrent then I agree they go into luni & luni-kernel. If we > >>> cannot > >>> then they are split into luni, concurrent, and concurrent-kernel. > >> > >> The more clean way for classlib code to use the atomic variables > >> would > >> be to create instances of the appropriate j.u.c.atomic classes, I > >> think. > >> It seems like the volatile variables can always just be declared with > >> the "volatile" keyword if needed. > >> > >>> > >>>> Assuming that there is a portion in this API set which > >>>> is VM-specific, it probably may make sense to put them into kernel. > >>> > >>> ack > >>> > >>>>> +1 for Atomics moving into a kernel. > >>>>> > >>>>> Same comment as above for atomics etc. not being left as unguarded > >>>>> public types/methods to avoid surprises from mischievous apps. > >>>> > >>>> Right, I would add the Atomics creation to the AccessorFactory > >>>> if we > >>>> agreed that all of this stuff is of the same nature. > >>> > >>> Again, if they are all in the same component, then one factory is ok > >>> (but unnecessary imho) -- if they are in different components > >>> then let's > >>> not couple them at this point. > >>> > >>> What do you think? > >> > >> I agree it would be good to not couple them. The problem would be how > >> to deal with the "informational methods" (see above) which are > >> supposed to be same for all accesses, for example - where the method > >> which would report fieldID's should be located if we are splitting > >> accessors between multiple packages according to VM-dependence rule. > >> > >> > >> Thanks, > >> Andrey. > >> > >> > >>> > >>> Tim > >>> > >>> -- > >>> > >>> Tim Ellison ([EMAIL PROTECTED]) > >>> IBM Java technology centre, UK. > >>> > >>> -------------------------------------------------------------------- > >>> - > >>> Terms of use : http://incubator.apache.org/harmony/mailing.html > >>> To unsubscribe, e-mail: [EMAIL PROTECTED] > >>> For additional commands, e-mail: harmony-dev- > >>> [EMAIL PROTECTED] > >>> > >>> > >> > >> > >> -- > >> Andrey Chernyshev > >> Intel Middleware Products Division > >> > >> --------------------------------------------------------------------- > >> Terms of use : http://incubator.apache.org/harmony/mailing.html > >> To unsubscribe, e-mail: [EMAIL PROTECTED] > >> For additional commands, e-mail: harmony-dev- > >> [EMAIL PROTECTED] > > > > > > --------------------------------------------------------------------- > > Terms of use : http://incubator.apache.org/harmony/mailing.html > > To unsubscribe, e-mail: [EMAIL PROTECTED] > > For additional commands, e-mail: [EMAIL PROTECTED] > > > > > --------------------------------------------------------------------- > Terms of use : http://incubator.apache.org/harmony/mailing.html > To unsubscribe, e-mail: [EMAIL PROTECTED] > For additional commands, e-mail: [EMAIL PROTECTED] --------------------------------------------------------------------- Terms of use : http://incubator.apache.org/harmony/mailing.html To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
