Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use valueOf() to box primitive values instead of creating new objects every time #88

Merged
merged 7 commits into from
Sep 27, 2022
Merged

Use valueOf() to box primitive values instead of creating new objects every time #88

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
d97f4a7
Use valueOf() to box primitive values instead of creating new objects…
rbasralian Sep 23, 2022
f3c0b4e
fix inconsistent spacing
rbasralian Sep 23, 2022
64ae0f3
pass args by value, not by reference. avoid `jvalue` where possible.
rbasralian Sep 23, 2022
f6ea799
revert accidental commit (change is in jpy_type.c)
rbasralian Sep 23, 2022
aa31470
rename macro. remove superfluous jvalue.
rbasralian Sep 24, 2022
9c0f459
don't use valueOf for values that will never be cached. rename macro …
rbasralian Sep 25, 2022
dd60b8a
revert back to always using valueOf when boxing. (boxed type construc…
rbasralian Sep 26, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
93 changes: 56 additions & 37 deletions src/main/c/jpy_jtype.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -401,53 +401,75 @@ int JType_CreateJavaObject_2(JNIEnv* jenv, JPy_JType* type, PyObject* pyArg, jcl
return 0;
}

#define JType_CALL_STATIC_OBJECT_METHOD_1_AND_RETURN(TARGET_TYPE, STATIC_METHOD_ID, ARG, JOBJECT_PTR) \
Py_BEGIN_ALLOW_THREADS; \
*JOBJECT_PTR = (*jenv)->CallStaticObjectMethod(jenv, TARGET_TYPE, STATIC_METHOD_ID, ARG); \
Py_END_ALLOW_THREADS; \
if (*objectRef == NULL) { \
PyErr_NoMemory(); \
return -1; \
} \
JPy_ON_JAVA_EXCEPTION_RETURN(-1); \
return 0;

#define JType_CREATE_JAVA_OBJECT_1_AND_RETURN(TARGET_TYPE, INIT_METHOD_ID, ARG, JOBJECT_PTR) \
Py_BEGIN_ALLOW_THREADS; \
*JOBJECT_PTR = (*jenv)->NewObject(jenv, TARGET_TYPE, INIT_METHOD_ID, ARG); \
Py_END_ALLOW_THREADS; \
if (*JOBJECT_PTR == NULL) { \
PyErr_NoMemory(); \
return -1; \
} \
JPy_ON_JAVA_EXCEPTION_RETURN(-1); \
return 0;


int JType_CreateJavaBooleanObject(JNIEnv* jenv, JPy_JType* type, PyObject* pyArg, jobject* objectRef)
{
jvalue value;
jboolean value;
if (PyBool_Check(pyArg) || JPy_IS_CLONG(pyArg)) {
value.z = JPy_AS_JBOOLEAN(pyArg);
value = JPy_AS_JBOOLEAN(pyArg);
} else {
return JType_PythonToJavaConversionError(type, pyArg);
}
return JType_CreateJavaObject(jenv, type, pyArg, JPy_Boolean_JClass, JPy_Boolean_Init_MID, value, objectRef);
JType_CALL_STATIC_OBJECT_METHOD_1_AND_RETURN(JPy_Boolean_JClass, JPy_Boolean_ValueOf_SMID, value, objectRef);
}

int JType_CreateJavaCharacterObject(JNIEnv* jenv, JPy_JType* type, PyObject* pyArg, jobject* objectRef)
{
jvalue value;
jchar value;
if (JPy_IS_CLONG(pyArg)) {
value.c = JPy_AS_JCHAR(pyArg);
value = JPy_AS_JCHAR(pyArg);
} else {
return JType_PythonToJavaConversionError(type, pyArg);
}
return JType_CreateJavaObject(jenv, type, pyArg, JPy_Character_JClass, JPy_Character_Init_MID, value, objectRef);
JType_CALL_STATIC_OBJECT_METHOD_1_AND_RETURN(JPy_Character_JClass, JPy_Character_ValueOf_SMID, value, objectRef);
}

int JType_CreateJavaByteObject(JNIEnv* jenv, JPy_JType* type, PyObject* pyArg, jobject* objectRef)
{
jvalue value;
jbyte value;
if (JPy_IS_CLONG(pyArg)) {
value.b = JPy_AS_JBYTE(pyArg);
value = JPy_AS_JBYTE(pyArg);
} else {
return JType_PythonToJavaConversionError(type, pyArg);
}
return JType_CreateJavaObject(jenv, type, pyArg, JPy_Byte_JClass, JPy_Byte_Init_MID, value, objectRef);
JType_CALL_STATIC_OBJECT_METHOD_1_AND_RETURN(JPy_Byte_JClass, JPy_Byte_ValueOf_SMID, value, objectRef);
}

int JType_CreateJavaShortObject(JNIEnv* jenv, JPy_JType* type, PyObject* pyArg, jobject* objectRef)
{
jvalue value;
jshort value;
if (JPy_IS_CLONG(pyArg)) {
value.s = JPy_AS_JSHORT(pyArg);
value = JPy_AS_JSHORT(pyArg);
} else {
return JType_PythonToJavaConversionError(type, pyArg);
}
return JType_CreateJavaObject(jenv, type, pyArg, JPy_Short_JClass, JPy_Short_Init_MID, value, objectRef);
JType_CALL_STATIC_OBJECT_METHOD_1_AND_RETURN(JPy_Short_JClass, JPy_Short_ValueOf_SMID, value, objectRef);
}

int JType_CreateJavaNumberFromPythonInt(JNIEnv* jenv, JPy_JType* type, PyObject* pyArg, jobject* objectRef)
{
jvalue value;
signed char b;
short s;
long i;
Expand All @@ -457,70 +479,67 @@ int JType_CreateJavaNumberFromPythonInt(JNIEnv* jenv, JPy_JType* type, PyObject*
s = (short) j;
b = (signed char) j;

// always create new objects for long/int/short; any values assigned
// assigned to those types will be outside the default JVM cache
// range of [-128, 127].
if (i != j) {
value.j = j;
return JType_CreateJavaObject(jenv, type, pyArg, JPy_Long_JClass, JPy_Long_Init_MID, value, objectRef);
JType_CREATE_JAVA_OBJECT_1_AND_RETURN(JPy_Long_JClass, JPy_Long_Init_MID, j, objectRef);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, it's not clear to me there will be any negligible performance difference if we keep doing valueOf. In fact, these constructors are deprecated. Not that I think they'll ever go away, but it might be worthwhile to stick w/ valueOf.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also don't know if it makes a difference, but valueOf is HotSpotIntrinsicCandidate (I don't know if constructors are or can be?).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, thanks, good point. I changed it back. Doesn't make sense to use a deprecated code for performance reasons if there's no evidence it helps performance

}
if (s != i) {
value.i = i;
return JType_CreateJavaObject(jenv, type, pyArg, JPy_Integer_JClass, JPy_Integer_Init_MID, value,
objectRef);
JType_CREATE_JAVA_OBJECT_1_AND_RETURN(JPy_Integer_JClass, JPy_Integer_Init_MID, i, objectRef);
}
if (b != s) {
value.s = s;
return JType_CreateJavaObject(jenv, type, pyArg, JPy_Short_JClass, JPy_Short_Init_MID, value,
objectRef);
JType_CREATE_JAVA_OBJECT_1_AND_RETURN(JPy_Short_JClass, JPy_Short_Init_MID, s, objectRef);
}
value.b = b;
return JType_CreateJavaObject(jenv, type, pyArg, JPy_Byte_JClass, JPy_Byte_Init_MID, value, objectRef);
JType_CALL_STATIC_OBJECT_METHOD_1_AND_RETURN(JPy_Byte_JClass, JPy_Byte_ValueOf_SMID, b, objectRef);
}

int JType_CreateJavaIntegerObject(JNIEnv* jenv, JPy_JType* type, PyObject* pyArg, jobject* objectRef)
{
jvalue value;
jint value;
if (JPy_IS_CLONG(pyArg)) {
value.i = JPy_AS_JINT(pyArg);
value = JPy_AS_JINT(pyArg);
} else {
return JType_PythonToJavaConversionError(type, pyArg);
}
return JType_CreateJavaObject(jenv, type, pyArg, JPy_Integer_JClass, JPy_Integer_Init_MID, value, objectRef);
JType_CALL_STATIC_OBJECT_METHOD_1_AND_RETURN(JPy_Integer_JClass, JPy_Integer_ValueOf_SMID, value, objectRef);
}

int JType_CreateJavaLongObject(JNIEnv* jenv, JPy_JType* type, PyObject* pyArg, jobject* objectRef)
{
jvalue value;
jlong value;
if (JPy_IS_CLONG(pyArg)) {
value.j = JPy_AS_JLONG(pyArg);
value = JPy_AS_JLONG(pyArg);
} else {
return JType_PythonToJavaConversionError(type, pyArg);
}
return JType_CreateJavaObject(jenv, type, pyArg, JPy_Long_JClass, JPy_Long_Init_MID, value, objectRef);
JType_CALL_STATIC_OBJECT_METHOD_1_AND_RETURN(JPy_Long_JClass, JPy_Long_ValueOf_SMID, value, objectRef);
}

int JType_CreateJavaFloatObject(JNIEnv* jenv, JPy_JType* type, PyObject* pyArg, jobject* objectRef)
{
jvalue value;
jfloat value;
if (JPy_IS_CLONG(pyArg)) {
value.f = (jfloat) JPy_AS_JLONG(pyArg);
value = (jfloat) JPy_AS_JLONG(pyArg);
} else if (PyFloat_Check(pyArg)) {
value.f = JPy_AS_JFLOAT(pyArg);
value = JPy_AS_JFLOAT(pyArg);
} else {
return JType_PythonToJavaConversionError(type, pyArg);
}
return JType_CreateJavaObject(jenv, type, pyArg, JPy_Float_JClass, JPy_Float_Init_MID, value, objectRef);
JType_CALL_STATIC_OBJECT_METHOD_1_AND_RETURN(JPy_Float_JClass, JPy_Float_ValueOf_SMID, value, objectRef);
}

int JType_CreateJavaDoubleObject(JNIEnv* jenv, JPy_JType* type, PyObject* pyArg, jobject* objectRef)
{
jvalue value;
jdouble value;
if (JPy_IS_CLONG(pyArg)) {
value.d = (jdouble) JPy_AS_JLONG(pyArg);
value = (jdouble) JPy_AS_JLONG(pyArg);
} else if (PyFloat_Check(pyArg)) {
value.d = JPy_AS_JDOUBLE(pyArg);
value = JPy_AS_JDOUBLE(pyArg);
} else {
return JType_PythonToJavaConversionError(type, pyArg);
}
return JType_CreateJavaObject(jenv, type, pyArg, JPy_Double_JClass, JPy_Double_Init_MID, value, objectRef);
JType_CALL_STATIC_OBJECT_METHOD_1_AND_RETURN(JPy_Double_JClass, JPy_Double_ValueOf_SMID, value, objectRef);
}

int JType_CreateJavaPyObject(JNIEnv* jenv, JPy_JType* type, PyObject* pyArg, jobject* objectRef)
Expand Down
24 changes: 24 additions & 0 deletions src/main/c/jpy_module.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,29 +194,37 @@ jclass JPy_StopIteration_JClass = NULL;
// java.lang.Boolean
jclass JPy_Boolean_JClass = NULL;
jmethodID JPy_Boolean_Init_MID = NULL;
jmethodID JPy_Boolean_ValueOf_SMID = NULL;
jmethodID JPy_Boolean_BooleanValue_MID = NULL;

jclass JPy_Character_JClass = NULL;
jmethodID JPy_Character_Init_MID;
jmethodID JPy_Character_ValueOf_SMID;
jmethodID JPy_Character_CharValue_MID = NULL;

jclass JPy_Byte_JClass = NULL;
jmethodID JPy_Byte_Init_MID = NULL;
jmethodID JPy_Byte_ValueOf_SMID = NULL;

jclass JPy_Short_JClass = NULL;
jmethodID JPy_Short_Init_MID = NULL;
jmethodID JPy_Short_ValueOf_SMID = NULL;

jclass JPy_Integer_JClass = NULL;
jmethodID JPy_Integer_Init_MID = NULL;
jmethodID JPy_Integer_ValueOf_SMID = NULL;

jclass JPy_Long_JClass = NULL;
jmethodID JPy_Long_Init_MID = NULL;
jmethodID JPy_Long_ValueOf_SMID = NULL;

jclass JPy_Float_JClass = NULL;
jmethodID JPy_Float_Init_MID = NULL;
jmethodID JPy_Float_ValueOf_SMID = NULL;

jclass JPy_Double_JClass = NULL;
jmethodID JPy_Double_Init_MID = NULL;
jmethodID JPy_Double_ValueOf_SMID = NULL;

// java.lang.Number
jclass JPy_Number_JClass = NULL;
Expand Down Expand Up @@ -891,31 +899,39 @@ int JPy_InitGlobalVars(JNIEnv* jenv)

DEFINE_CLASS(JPy_Boolean_JClass, "java/lang/Boolean");
DEFINE_METHOD(JPy_Boolean_Init_MID, JPy_Boolean_JClass, "<init>", "(Z)V");
DEFINE_STATIC_METHOD(JPy_Boolean_ValueOf_SMID, JPy_Boolean_JClass, "valueOf", "(Z)Ljava/lang/Boolean;");
DEFINE_METHOD(JPy_Boolean_BooleanValue_MID, JPy_Boolean_JClass, "booleanValue", "()Z");

DEFINE_CLASS(JPy_Character_JClass, "java/lang/Character");
DEFINE_METHOD(JPy_Character_Init_MID, JPy_Character_JClass, "<init>", "(C)V");
DEFINE_STATIC_METHOD(JPy_Character_ValueOf_SMID, JPy_Character_JClass, "valueOf", "(C)Ljava/lang/Character;");
DEFINE_METHOD(JPy_Character_CharValue_MID, JPy_Character_JClass, "charValue", "()C");

DEFINE_CLASS(JPy_Number_JClass, "java/lang/Number");

DEFINE_CLASS(JPy_Byte_JClass, "java/lang/Byte");
DEFINE_METHOD(JPy_Byte_Init_MID, JPy_Byte_JClass, "<init>", "(B)V");
DEFINE_STATIC_METHOD(JPy_Byte_ValueOf_SMID, JPy_Byte_JClass, "valueOf", "(B)Ljava/lang/Byte;");

DEFINE_CLASS(JPy_Short_JClass, "java/lang/Short");
DEFINE_METHOD(JPy_Short_Init_MID, JPy_Short_JClass, "<init>", "(S)V");
DEFINE_STATIC_METHOD(JPy_Short_ValueOf_SMID, JPy_Short_JClass, "valueOf", "(S)Ljava/lang/Short;");

DEFINE_CLASS(JPy_Integer_JClass, "java/lang/Integer");
DEFINE_METHOD(JPy_Integer_Init_MID, JPy_Integer_JClass, "<init>", "(I)V");
DEFINE_STATIC_METHOD(JPy_Integer_ValueOf_SMID, JPy_Integer_JClass, "valueOf", "(I)Ljava/lang/Integer;");

DEFINE_CLASS(JPy_Long_JClass, "java/lang/Long");
DEFINE_METHOD(JPy_Long_Init_MID, JPy_Long_JClass, "<init>", "(J)V");
DEFINE_STATIC_METHOD(JPy_Long_ValueOf_SMID, JPy_Long_JClass, "valueOf", "(J)Ljava/lang/Long;");

DEFINE_CLASS(JPy_Float_JClass, "java/lang/Float");
DEFINE_METHOD(JPy_Float_Init_MID, JPy_Float_JClass, "<init>", "(F)V");
DEFINE_STATIC_METHOD(JPy_Float_ValueOf_SMID, JPy_Float_JClass, "valueOf", "(F)Ljava/lang/Float;");

DEFINE_CLASS(JPy_Double_JClass, "java/lang/Double");
DEFINE_METHOD(JPy_Double_Init_MID, JPy_Double_JClass, "<init>", "(D)V");
DEFINE_STATIC_METHOD(JPy_Double_ValueOf_SMID, JPy_Double_JClass, "valueOf", "(D)Ljava/lang/Double;");

DEFINE_CLASS(JPy_Number_JClass, "java/lang/Number");
DEFINE_METHOD(JPy_Number_IntValue_MID, JPy_Number_JClass, "intValue", "()I");
Expand Down Expand Up @@ -1037,15 +1053,23 @@ void JPy_ClearGlobalVars(JNIEnv* jenv)
JPy_Field_GetModifiers_MID = NULL;
JPy_Field_GetType_MID = NULL;
JPy_Boolean_Init_MID = NULL;
JPy_Boolean_ValueOf_SMID = NULL;
JPy_Boolean_BooleanValue_MID = NULL;
JPy_Character_Init_MID = NULL;
JPy_Character_ValueOf_SMID = NULL;
JPy_Character_CharValue_MID = NULL;
JPy_Byte_Init_MID = NULL;
JPy_Short_Init_MID = NULL;
JPy_Integer_Init_MID = NULL;
JPy_Long_Init_MID = NULL;
JPy_Float_Init_MID = NULL;
JPy_Double_Init_MID = NULL;
JPy_Byte_ValueOf_SMID = NULL;
JPy_Short_ValueOf_SMID = NULL;
JPy_Integer_ValueOf_SMID = NULL;
JPy_Long_ValueOf_SMID = NULL;
JPy_Float_ValueOf_SMID = NULL;
JPy_Double_ValueOf_SMID = NULL;
JPy_Number_IntValue_MID = NULL;
JPy_Number_LongValue_MID = NULL;
JPy_Number_DoubleValue_MID = NULL;
Expand Down
8 changes: 8 additions & 0 deletions src/main/c/jpy_module.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,31 +216,39 @@ extern jclass JPy_StopIteration_JClass;

extern jclass JPy_Boolean_JClass;
extern jmethodID JPy_Boolean_Init_MID;
extern jmethodID JPy_Boolean_ValueOf_SMID;
extern jmethodID JPy_Boolean_BooleanValue_MID;

extern jclass JPy_Character_JClass;
extern jmethodID JPy_Character_Init_MID;
extern jmethodID JPy_Character_ValueOf_SMID;
extern jmethodID JPy_Character_CharValue_MID;

extern jclass JPy_Number_JClass;

extern jclass JPy_Byte_JClass;
extern jmethodID JPy_Byte_Init_MID;
extern jmethodID JPy_Byte_ValueOf_SMID;

extern jclass JPy_Short_JClass;
extern jmethodID JPy_Short_Init_MID;
extern jmethodID JPy_Short_ValueOf_SMID;

extern jclass JPy_Integer_JClass;
extern jmethodID JPy_Integer_Init_MID;
extern jmethodID JPy_Integer_ValueOf_SMID;

extern jclass JPy_Long_JClass;
extern jmethodID JPy_Long_Init_MID;
extern jmethodID JPy_Long_ValueOf_SMID;

extern jclass JPy_Float_JClass;
extern jmethodID JPy_Float_Init_MID;
extern jmethodID JPy_Float_ValueOf_SMID;

extern jclass JPy_Double_JClass;
extern jmethodID JPy_Double_Init_MID;
extern jmethodID JPy_Double_ValueOf_SMID;

extern jclass JPy_Number_JClass;
extern jmethodID JPy_Number_IntValue_MID;
Expand Down