Bug - Fixed Race condition between multiple matchers executing in parallel

[Veracity]

Ah. So the issue is that a LibraryFunction defines its variable references at construction time like this:

		for ( int i = 1; i <= params.length; ++i )
		{
			Variable variable = new Variable( params[ i - 1 ] );
			this.variableReferences.add( new VariableReference( variable ) );
			args[ i ] = Value.class;
		}

Note that it makes up the names of the function arguments, since the actual method for a LibraryFunction does not care.

Very interesting. I would refactor things like this:

Currently: FunctionCall:

saveBindings
iterate over parameters and build an array of values
iterate over values and bind them to variable references
call target.execute
restoreBindings

UserDefinedFunction:

(has saveBindings and restoreBindings)

execute simply executes the scope

LibraryFunction
(has no saveBindings and RestoreBindings)

execute iterates over variable references and makes an array of values - exactly like the one built by FunctionCall
invokes the method

Seems to me it could do this:


FunctionCall:

iterate over parameters and build an array of values
call target.execute( values )

UserDefinedFunction:

saveBindings
iterate over values and bind them to variable references
executes the scope
restoreBindings

LibraryFunction

invokes the method with values array

In other words, bind variable references only for UserDefinedFunction, don't duplicate building values array, and so on.

I'll do it. How hard can it be?

 

[roippi]

Yep, sounds good. Also sounds like the type of refactoring that I wasn't up for doing!

Also,

The performance hit happens even if only one thread calls the function. That's why you don't pre-emptively synchronize everything, after all.

If you're talking about the "performance hit" of a single uncontested thread obtaining an object's monitor, that "performance hit" is similar to that of multiplying two integers together. Or perhaps less, depending on JVM optimization. i.e. not worth talking about. There are plenty of reasons not to extraneously include synchronization in code, but single-threaded performance is not even in the top 10.

 

[Veracity]

Try 12016.

 

[roippi]

All good now.

 

[xKiv]

If you're talking about the "performance hit" of a single uncontested thread obtaining an object's monitor, that "performance hit" is similar to that of multiplying two integers together. Or perhaps less, depending on JVM optimization. i.e. not worth talking about. There are plenty of reasons not to extraneously include synchronization in code, but single-threaded performance is not even in the top 10.

Well, apparently, there are other issues too (like forcing variables to "synchronize", which might prevent some optimalizations) ... but a little "light" reading suggests that the "performance" hit hasn't been a significant issue since java 1.4.

On the other hand, I now think UserDefinedFunction must *not* be synchronized, at all - otherwise any recursion would result in a deadlock.

EAT: oh, and, if I understand it right, the "performance hit" on modern JVMs isn't that bad even with contested access ...

 

[Veracity]

On the other hand, I now think UserDefinedFunction must *not* be synchronized, at all - otherwise any recursion would result in a deadlock.

Synchronization prevents multiple threads from entering a critical section. Recursion happens within a single thread.

 

[roippi]

Yeah, synchronization is reentrant. (As are ReentrantLocks in the Locks package, shockingly.) A thread can acquire a lock that it already possesses.

 

[xKiv]

I don't know what I was thinking.