Re: [Jython-dev] PyType publication with better concurrency

[Jeff A. <ja...@fa...>]

Jim, all:

By a series of small steps I've come quite a long way in this aspect of 
PyType. It might be worth a look.
https://bitbucket.org/tournesol/jython-fgtype/commits/3fc5d8ff48bf5d20cb57169cd3f43a3cd90df0cd

In this last commit I've entirely dispensed with any long-term reference 
to the created PyType in the type registry: only the ClassValue holds 
one. The explicit management of reachability goes away and we have just 
the behaviour given us by ClassValue. The only down-side I see is that a 
PyType created for a Java class you handle briefly from Python will not 
be collectable until the class is.

The old test for weakness, which I've been running in parallel with the 
new, requires PyType.getClassToType(), and can no longer be supported. 
(You can't enumerate the ClassValue.) I believe the replacement tests 
the same thing -- they used to pass and fail together.

I've moved some actions around, and removed some of the checks. We 
seemingly expected the desired PyType to be created at any moment by a 
side effect. Reason and regression testing tell me that doesn't happen 
in the new arrangement. I think there is slightly less code to execute, 
but masses more comment and debug -- it's my way of figuring it out. 
It's all very educational.

I still don't understand:

 1. Whether we find the downside (PyType not GC'd until the class is)
    important.
 2. Whether I have broken something that the regression tests don't
    cover (like use with isolating class loaders).
 3. What needsInners is really up to. I see *what* it does, but not
    *why* well enough to consider alternatives. It would simplify
    computeValue if there were no fromClassSkippingInners: could
    needsInners be a registry global? And why do we skip processing
    them, when we do?


Jeff

On 09/08/2017 08:24, Jeff Allen wrote:
> Jim, all:
>
> Apologies for the near repeat, but my first attempt to give this a 
> life of its own on Jython-dev ended up nested under issue 2487 again. 
> Hope this lands as a fresh topic.
>
> On 05/08/2017 20:34, in Issue #2387 
> (http://bugs.jython.org/issue2487), Jim Baker wrote:
>>
>> What if instead of current one-level cache, we had a two-level cache?
>> Level 1 uses weak key/weak value for the class/type entries (C, T);
>> (C, T) entries that are expired from Level 1 are moved to Level 2;
>> Level 2 uses an expiration time so a ClassLoader could be unloaded in
>> say some reasonable amount of time.
>> ...
>>
>> Lastly, 
>> https://docs.oracle.com/javase/7/docs/api/java/lang/ClassValue.html
>> looks potentially useful as an alternative for publishing PyType
>> objects into a cache. But we need to address two key questions:
>>
>> 1. Whether ClassValue#computeValue would work properly in the
>>     re-entrant case where the class graph from a given class C is
>>     explored, as is done for inners, possibly referring back to C. This
>>     is why we could not get the publication of the type in the map done
>>     after the init (as one would usually want to do!) in
>> https://github.com/jythontools/jython/blob/master/src/org/python/core/PyType.java#L1515 
>>
>>     (putIfAbsent); class C would be referenced, and it would attempt to
>>     build again (and stack overflow). No solution I tried could break
>>     this problem.
>>
>> 2. When to call ClassValue#removeValue ! We still have to keep track
>>     of the cache with respect to PyType.
>
> I have an idea that I can use ClassValue with this, but it sits as a 
> (lock-free) cache in front of the same (properly locked) apparatus we 
> have now.
> ...
> Thing is, I don't understand in what circumstances we ought to create 
> the strong reference: what need are we actually seeking to meet? Is 
> the critical use case that the PyType could be freed when Python code 
> stops using it, and yet the Java class has reason to remain?