harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Tim Ellison <t.p.elli...@gmail.com>
Subject Re: [classlib][vmi] VMI classes for Thread/Object manipulation for java.util.concurrent
Date Tue, 26 Sep 2006 10:36:02 GMT
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.

That's the right approach.

> 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

Yep, the volatile and atomic operations are VM specific (need to go into
luni-kernel); the misc module only has portable methods at the moment.

> 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.

Ok, so let's agree that the code in misc gets merged into kernel, and we
drop the idea of a concurrency-kernel.  The only question that remains
open for me is whether we need to split the accessors into common
(JNI-based) methods and kernel (VM-specific) methods.

It sounds like you prefer not to split them, which would put an extra
burden on the VM-port to implement more stuff.  If they were split then
the consumer (classlib developer) would/may have to use both types.

> * 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.

yep, go ahead and move them and remove the 'misc' module.

> 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.

I just ask you to consider the kernel / non-kernel split.  When they go
into kernel they are no longer shared code (and any fixes or changes we
need to put in there have to be propagated to each VM).


> -Nathan
>> -----Original Message-----
>> From: Andrey Chernyshev [mailto:a.y.chernyshev@gmail.com]
>> 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 <t.p.ellison@gmail.com> wrote:
>>> Andrey Chernyshev wrote:
>>>> On 9/20/06, Tim Ellison <t.p.ellison@gmail.com> 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 (t.p.ellison@gmail.com)
>>> IBM Java technology centre, UK.
>>> ---------------------------------------------------------------------
>>> Terms of use : http://incubator.apache.org/harmony/mailing.html
>>> To unsubscribe, e-mail: harmony-dev-unsubscribe@incubator.apache.org
>>> For additional commands, e-mail: harmony-dev-help@incubator.apache.org
>> --
>> Andrey Chernyshev
>> Intel Middleware Products Division
>> ---------------------------------------------------------------------
>> Terms of use : http://incubator.apache.org/harmony/mailing.html
>> To unsubscribe, e-mail: harmony-dev-unsubscribe@incubator.apache.org
>> For additional commands, e-mail: harmony-dev-help@incubator.apache.org
> ---------------------------------------------------------------------
> Terms of use : http://incubator.apache.org/harmony/mailing.html
> To unsubscribe, e-mail: harmony-dev-unsubscribe@incubator.apache.org
> For additional commands, e-mail: harmony-dev-help@incubator.apache.org


Tim Ellison (t.p.ellison@gmail.com)
IBM Java technology centre, UK.

Terms of use : http://incubator.apache.org/harmony/mailing.html
To unsubscribe, e-mail: harmony-dev-unsubscribe@incubator.apache.org
For additional commands, e-mail: harmony-dev-help@incubator.apache.org

View raw message