On 07.08.23 15:15, Gillespie, Oli wrote:
Hi Jochen,
Thanks for looking, and for the findings - interesting!
Actually I think my reproducer was not a good representation of the real issue
I saw in production.
I added the getName() call in my first repro as a random thing to do with the
argument, but that introduced unintended behaviour.
It did help uncovering a problem. So that was good in itself.
My real case is closer to this (literally implementations of List, in my case):
```
def foo(List<String> x) { }
def bar(List<String> x) { foo(x) }
List<String> l1 = new ArrayList();
List<String> l2 = new LinkedList();
while (true) {
bar(l1)
127.times { bar(l2) }
}
```
The call site causing the difficulty here is (I think) foo(x) within bar, where
a 'sameClasses' guard is inserted and
invalidates the previous method handle whenever the argument class changes
(LinkedList != ArrayList) -
I confirmed this by adding logging to the guard.
I think you are almost correct. For the handle itself you are. But the
handle was really intended to work that way. in front of the handle
there is a LRU map cache, taking the receiver as key. So bar(l1) will
install one handle in that cache und bar(l2) will install a new handle,
replacing the old handle. Because bar(l1) is lost, it gets recreated,
leading to bar(l2) getting lost.
I think that cache needs a bit of rethinking. Most of the guards should
move from the handle to the cache I think.
Right now we have basically (very much simplified)
MH handle = cache.get(receiver.getClass(),
()->createNewHandleForNewSituation())
handle.invoke(args)
And the handle has basically the structure (ignoring null receiver):
switchPointGuard *
GroovyRuntimeExceptionUnwrapper * argumentClassesOrMetaClassGuard *
transformArguments * directInvoker
Where each guard ultimately lead back to the cache.
And I think it should be more like this:
switchPointGuard * GroovyRuntimeExceptionUnwrapper * cacheGet
And cacheGet get through a list of handle
argumentClassesOrMetaClassGuard, where the success branch leads to
transformArguments * directInvoker and the failure leads to the next
handle guard, or the creation of a new guard and handle. With the
ability that the list is in LRU order
But I did not really experiment with this structure, so I do not know if
that kind of handle is a good idea or not.
And something that would also have to be tested is if the fallback used
by all the guards should not be a general invoker, that does invocation
using the meta class and the specialization is only done after a
threshold has been reached.
I am pretty sure there are also better constructions. But I very much
would like to get away from the map with synchronization and instead use
more persistent collections. Plus not all elements of the handle are
always required. Especially that GroovyRuntimeUnwrapper is bad for
performance.
bye Jochen
