You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@uima.apache.org by Marshall Schor <ms...@schor.com> on 2007/07/11 22:05:50 UTC

multi-threading and TypeSystemImpl

I poked around in the code for TypeSystemImpl a bit and concluded:

1) Re: The Array of arbitrary FSs mechanism, which is implemented to 
allow adding additional arrays of specific FS types after the type 
system has been committed.  The call which actually adds a type is 
ts.getArrayType(componentType).  The framework seems to avoid calling 
this, other than when creating a whole type system.  But this method is 
public, defined in the TypeSystem interfaqce, and so a user could call 
it at any time, on any thread:

  /**
   * Obtain an array type with component type <code>componentType</code>.
   *
   * @param componentType
   *          The type of the elements of the resulting array type. This 
can be any type, even
   *          another array type.
   * @return The array type with the corresponding component type.
   */
  Type getArrayType(Type componentType);

This method will do a lookup in componentToArrayTypeMap, and if it 
doesn't find the component type, it will add it, and update this using a 
"put".  The componentToArrayTypeMap is a non-synchronized 
IntRedBlackTree.  So this is a potential failure if this happens in a 
multi-threaded environment, via a user calling ts.getArrayType(...).

We probably need to make access to this synchronized, or use some 
fancier method of locking.

2) It seems very surprising to me that the synchronization around 
get(int) would ever have a collision - it's too tiny a piece of code.  
The code for get(int) I think just is an array dereference, contained in 
a synchronized method.  So - I think something else is causing the 
slowdown, but I don't know what that could be.  It also could be that 
the JProbe code is altering the behavior.

3) We should probably eliminate the call in ll_isArrayType that calls 
ll_isValidTypeCode - it's not needed.

4) I think we want to design the TypeSystemImpl so that when it is 
"locked" - it is thread-safe for running with multiple threads.  This 
seems to involve adding synchronization to other object accesses.  
(Example object: the "locked" boolean).  Because of (2) above, I'm 
guessing (hoping) this would not have an impact on performance.  Before 
it is locked, it can probably be assumed that only one thread will ever 
be accessing the type system (is this true / "provable" in the current 
design?  - if not - I suppose we could make the design thread-safe even 
before "committting").  

5) The instance of CASMetaData  and FSClassRegistry are tied to 
instances of TypeSystemImpl, and so, also has to be thread safe.

Note that the FSClassRegistry "generators" already have shadow instances 
for each CAS in a CAS Pool that might be running
on different threads.

If we did all this, we still probably haven't solved the problem of 
slowdown for xmi serialization.  Would be great to have a test case for 
this.  Greg - can you sanitize up something simple that shows the 
problem, and then submit a Jira issue and attach the test as a patch?

-Marshall








Re: multi-threading and TypeSystemImpl

Posted by Adam Lally <al...@alum.rpi.edu>.
On 7/12/07, Thilo Goetz <tw...@gmx.de> wrote:
> >> :-) So the whole concept is useless.  Remind me why we
> >> have parametric arrays?
> >>
> > Two uses currently:  One is XMI serialization - it makes use of this
> > info for a much more compact serialized form. The second: JCasGen uses
> > this info when generating cover functions to do compile-time checking of
> > arguments, and returning the right class of result.  So, if you have an
> > FSArray of Foo objects, defined as the type:feature MyType:FooArray, the
> > setters and getters for elements of this type,
> > anInstanceOfMyType.setFooArray(index, value) has the method parameter
> > type for "value" be of class Foo, rather than of class Top, and
> > anInstanceOfMyType.getFooArray(index) returns an instance of Foo class.
> >
>
> Shouldn't we have this capability on the regular CAS as well, then?
>

Sure, but how should we proceed?  The current state of things is that
type system descriptors can specify an <elementType> for an
array-valued feature, for example:

        <featureDescription>
          <name>annotationArray</name>
          <rangeTypeName>uima.cas.FSArray</rangeTypeName>
          <elementType>uima.tcas.Annotation</elementType>
        </featureDescription>

JCAS and XMI serialization essentially just treat this as metadata
attached to the feature.  There are never actually any instances of
the parameterized array object.  Partly the idea was that people could
just document the elementTypes that they were currently implicitly
using, and the wouldn't have to change their code.  (In some ways this
sort of reminds me of the Java 5 generics.)

This is why subsumes is the way that it is.  It allows assigning a
"plain" FSArray to a feature with range type uima.tcas.Annotation[].

It would not be good to change this subsumes behavior now.

A possible way out if we want to implement real parameterized arrays
is for people to declare the feature with the square-bracket array
notation:
        <featureDescription>
          <name>annotationArray</name>
          <rangeTypeName>uima.tcas.Annotation[]</rangeTypeName>
        </featureDescription>

This would then not allow you to assign a plain FSArray to this feature.

For the current users of <elementType> we can make this really be just
metadata attached to the Feature, rather than impacting the actual
range type of the feature, and existing code should not break.

But then there's all the implications of creating new Types after the
TypeSystem is "locked", which we need to think through.

For 2.2 I am in favor of deprecating getArrayType() since it is
not-useful as-is, and possibly un-deprecating it in 2.3 after the
whole parameterized array thing works the way we want it to.

-Adam

Re: multi-threading and TypeSystemImpl

Posted by Thilo Goetz <tw...@gmx.de>.
Marshall Schor wrote:
> Thilo Goetz wrote:
>> Adam Lally wrote:
>>  
>>> On 7/12/07, Thilo Goetz <tw...@gmx.de> wrote:
>>>    
>>>> What's unclear about this method?
>>>>
>>>>       
>>> You can get a Type object that represents a typed-array, but there is
>>> no way to create an instance of such an array.  What good is it then
>>> to get the Type object?
>>>
>>> -Adam
>>>     
>>
>> :-) So the whole concept is useless.  Remind me why we
>> have parametric arrays?
>>   
> Two uses currently:  One is XMI serialization - it makes use of this
> info for a much more compact serialized form. The second: JCasGen uses
> this info when generating cover functions to do compile-time checking of
> arguments, and returning the right class of result.  So, if you have an
> FSArray of Foo objects, defined as the type:feature MyType:FooArray, the
> setters and getters for elements of this type, 
> anInstanceOfMyType.setFooArray(index, value) has the method parameter
> type for "value" be of class Foo, rather than of class Top, and
> anInstanceOfMyType.getFooArray(index) returns an instance of Foo class.
> 

Shouldn't we have this capability on the regular CAS as well, then?

> Perhaps there are other uses, but those come to mind right now. -Marshall
>> --Thilo
>>
>>
>>
>>   


Re: multi-threading and TypeSystemImpl

Posted by Marshall Schor <ms...@schor.com>.
Thilo Goetz wrote:
> Adam Lally wrote:
>   
>> On 7/12/07, Thilo Goetz <tw...@gmx.de> wrote:
>>     
>>> What's unclear about this method?
>>>
>>>       
>> You can get a Type object that represents a typed-array, but there is
>> no way to create an instance of such an array.  What good is it then
>> to get the Type object?
>>
>> -Adam
>>     
>
> :-) So the whole concept is useless.  Remind me why we
> have parametric arrays?
>   
Two uses currently:  One is XMI serialization - it makes use of this 
info for a much more compact serialized form. 
The second: JCasGen uses this info when generating cover functions to do 
compile-time checking of arguments, and returning the right class of 
result.  So, if you have an FSArray of Foo objects, defined as the 
type:feature MyType:FooArray, the setters and getters for elements of 
this type,  anInstanceOfMyType.setFooArray(index, value) has the method 
parameter type for "value" be of class Foo, rather than of class Top, 
and anInstanceOfMyType.getFooArray(index) returns an instance of Foo class.

Perhaps there are other uses, but those come to mind right now. -Marshall
> --Thilo
>
>
>
>   


Re: multi-threading and TypeSystemImpl

Posted by Thilo Goetz <tw...@gmx.de>.

Adam Lally wrote:
> On 7/12/07, Thilo Goetz <tw...@gmx.de> wrote:
>> What's unclear about this method?
>>
> 
> You can get a Type object that represents a typed-array, but there is
> no way to create an instance of such an array.  What good is it then
> to get the Type object?
> 
> -Adam

:-) So the whole concept is useless.  Remind me why we
have parametric arrays?

--Thilo


Re: multi-threading and TypeSystemImpl

Posted by Adam Lally <al...@alum.rpi.edu>.
On 7/12/07, Thilo Goetz <tw...@gmx.de> wrote:
> What's unclear about this method?
>

You can get a Type object that represents a typed-array, but there is
no way to create an instance of such an array.  What good is it then
to get the Type object?

-Adam



> Adam Lally wrote:
> > On 7/12/07, Thilo Goetz <tw...@gmx.de> wrote:
> >> Before we start deprecating methods, though, I would prefer it if we
> >> sat down and thought the whole array story through.  What we have now
> >> could certainly use improvement, but I would like to understand what
> >> the real design is that we're aiming at.
> >
> > I think it is okay to deprecate first and understand later.  A method
> > that we don't understand all the implications of and which we may want
> > to change later is a good one to discourage users from using today.
> >
> > -Adam
>

Re: multi-threading and TypeSystemImpl

Posted by Thilo Goetz <tw...@gmx.de>.
What's unclear about this method?

Adam Lally wrote:
> On 7/12/07, Thilo Goetz <tw...@gmx.de> wrote:
>> Before we start deprecating methods, though, I would prefer it if we
>> sat down and thought the whole array story through.  What we have now
>> could certainly use improvement, but I would like to understand what
>> the real design is that we're aiming at.
> 
> I think it is okay to deprecate first and understand later.  A method
> that we don't understand all the implications of and which we may want
> to change later is a good one to discourage users from using today.
> 
> -Adam

Re: multi-threading and TypeSystemImpl

Posted by Adam Lally <al...@alum.rpi.edu>.
On 7/12/07, Thilo Goetz <tw...@gmx.de> wrote:
> Before we start deprecating methods, though, I would prefer it if we
> sat down and thought the whole array story through.  What we have now
> could certainly use improvement, but I would like to understand what
> the real design is that we're aiming at.

I think it is okay to deprecate first and understand later.  A method
that we don't understand all the implications of and which we may want
to change later is a good one to discourage users from using today.

-Adam

Re: multi-threading and TypeSystemImpl

Posted by Thilo Goetz <tw...@gmx.de>.
Adam Lally wrote:
> On 7/11/07, Marshall Schor <ms...@schor.com> wrote:
>> 4) I think we want to design the TypeSystemImpl so that when it is
>> "locked" - it is thread-safe for running with multiple threads.  This
>> seems to involve adding synchronization to other object accesses.
>> (Example object: the "locked" boolean).  Because of (2) above, I'm
>> guessing (hoping) this would not have an impact on performance.
> 
> Assuming for a second that "locked" really meant that the type system
> didn't change, it seems like there should be some more efficient way
> to do this.  A thread could synchronize briefly on the TypeSystem once
> when it first begins to process a CAS (just to make sure there is a
> "write barrier" so the thread really sees all the changes done prior
> to the locking), and thereafter it does not need to synchronize every
> time it wants to query anything.
> 
> Even without much contention for the lock, synchronization adds some
> overhead.  I guess I've heard this is a lot less in recent JRE
> versions than it used to be, but I still worry.
> 
> However, given the situation with the array types allowed to be
> created after the type system is locked, this gets a lot more
> complicated.  I'm not sure there's any good way to do it other than to
> stop creating new types after the type system is locked.  Like you
> said, Marshall, the framework itself never does this, but exposes a
> public method that could.  Maybe we need to deprecate that method?
> 
> -Adam

If we don't have this method, users can only use those array types that
were used in a descriptor somewhere.  So you can't create an array of
tokens, for example, if it wasn't mentioned in a descriptor.  That
would be contrary to my expectations, but I don't really care too much,
either.

Before we start deprecating methods, though, I would prefer it if we
sat down and thought the whole array story through.  What we have now
could certainly use improvement, but I would like to understand what
the real design is that we're aiming at.  For example, we still have
this comment in the type system impl (and the code that goes with it):

    // Yes, the code below is intentional. Until we actually support real
    // arrays of some
    // particular fs,
    // we have FSArray is the supertype of xxxx[] AND
    // xxx[] is the supertype of FSArray
    // (this second relation because all we can generate are instances of
    // FSArray
    // and we must be able to assign them to xxx[] )

--Thilo


Re: multi-threading and TypeSystemImpl

Posted by Adam Lally <al...@alum.rpi.edu>.
On 7/11/07, Marshall Schor <ms...@schor.com> wrote:
> 4) I think we want to design the TypeSystemImpl so that when it is
> "locked" - it is thread-safe for running with multiple threads.  This
> seems to involve adding synchronization to other object accesses.
> (Example object: the "locked" boolean).  Because of (2) above, I'm
> guessing (hoping) this would not have an impact on performance.

Assuming for a second that "locked" really meant that the type system
didn't change, it seems like there should be some more efficient way
to do this.  A thread could synchronize briefly on the TypeSystem once
when it first begins to process a CAS (just to make sure there is a
"write barrier" so the thread really sees all the changes done prior
to the locking), and thereafter it does not need to synchronize every
time it wants to query anything.

Even without much contention for the lock, synchronization adds some
overhead.  I guess I've heard this is a lot less in recent JRE
versions than it used to be, but I still worry.

However, given the situation with the array types allowed to be
created after the type system is locked, this gets a lot more
complicated.  I'm not sure there's any good way to do it other than to
stop creating new types after the type system is locked.  Like you
said, Marshall, the framework itself never does this, but exposes a
public method that could.  Maybe we need to deprecate that method?

-Adam