Skip to content

Conversation

@yrodiere
Copy link
Member

yrodiere commented Aug 2, 2023

@sebersole Do you remember the problem that prevented this PR to move further towards completion? I'm afraid I don't...

Regarding the (non-default) implementation of createTypedArray(), of course the most reliable and most performant solution would be to implement the method for every single JavaType.

But if that's not practical, that's not strictly necessary as far as Quarkus is concerned. From what I can see JavaType instances are created about when Hibernate ORM creates the Metadata, so it could be enough to perform reflective operations in the constructor of AbstractClassJavaType. For example something like this could work for Quarkus (I'd need to test of course):

public abstract class AbstractClassJavaType<T> implements BasicJavaType<T>, Serializable {	private final Class<T> type;	private final MutabilityPlan<T> mutabilityPlan;	private final Comparator<T> comparator; private final Class<T[]> arrayType; private final Constructor<T[]> arrayTypeConstructor;	protected AbstractClassJavaType(	Class<? extends T> type,	MutabilityPlan<? extends T> mutabilityPlan,	Comparator<? extends T> comparator) {	this.type = (Class<T>) type;	this.mutabilityPlan = (MutabilityPlan<T>) mutabilityPlan;	this.comparator = (Comparator<T>) comparator;	// Reflective operation, but happens during metadata building	// (before native compilation in Quarkus) so it's fine	this.arrayType = (Class<T[]>) Array.newInstance( getJavaTypeClass(), 0 ).getClass();	this.arrayTypeConstructor = arrayType.getConstructor( int.class );	}	@Override	public T[] createTypedArray(int numberOfElements) {	return arrayTypeConstructor.newInstance( numberOfElements );	}	@Override	public Class<T[]> getArrayType() {	return arrayType;	} 

This solution would probably lead to some overhead, since I suspect JavaType instances live on the heap as long as the Metadata/SessionFactory are there, so GraalVM would probably consider all types can potentially be reached and wouldn't eliminate any dead code. But it should work, so it's a reasonable trade-off if implementing createTypedArray() for every single JavaType subclass is not an option.

Not sure we can do anything about org.hibernate.type.descriptor.java.AbstractJavaType though... We'd have to implement methods for each subclass.

@beikov
Copy link
Member

beikov commented Aug 2, 2023

I think we concluded that we'd like to try using Object[] for everything instead of specialized arrays. In the meantime, Quarkus should register array types for all basic types for reflection.

@sebersole
Copy link
Member Author

The main problem I ran into was JavaType descriptors for primitive/wrapper types. IntegerJavaType e.g. handles both int and Integer. So a proper implementation would need to know which to use, which would need to get passed in - which gets fugly

@gavinking
Copy link
Member

Looks like this can be closed, is that correct, @sebersole ?

@yrodiere
Copy link
Member

yrodiere commented Jan 2, 2025

The main problem I ran into was JavaType descriptors for primitive/wrapper types. IntegerJavaType e.g. handles both int and Integer. So a proper implementation would need to know which to use, which would need to get passed in - which gets fugly

From what I can see, most use cases where we need to create an array, actually require an Object[] already, which cannot be an array of primitive types (since e.g. int[] cannot be assigned to Object[]).

So... I'll incorporate what you have in my attempt to fix HHH-18976, and will follow @beikov's suggestion:

I think we concluded that we'd like to try using Object[] for everything instead of specialized arrays. In the meantime, Quarkus should register array types for all basic types for reflection.

@yrodiere
Copy link
Member

yrodiere commented Jan 2, 2025

and will follow @beikov's suggestion:

I think we concluded that we'd like to try using Object[] for everything instead of specialized arrays. In the meantime, Quarkus should register array types for all basic types for reflection.

Actually I misread Christian's suggestion. What I meant is that I'll use JavaType#createArray wherever possible, which is wherever the array is manipulated as an Object[], which should already give us some improvement -- and, critically, get me unstuck when it comes to HHH-18976.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

4 participants