RFR: aaload/aastore profiling

[Roland Westrelin]

Hi Tobias,

Thanks for reviewing this.

> - c1_Runtime1.cpp:476, can you move this into the else branch (line 485) to avoid the is_flattened()
> check and also move the assert into the two branches?

This?

diff -r f81c768ad125 src/hotspot/share/c1/c1_Runtime1.cpp
--- a/src/hotspot/share/c1/c1_Runtime1.cpp	Wed Nov 13 17:51:40 2019 +0100
+++ b/src/hotspot/share/c1/c1_Runtime1.cpp	Fri Nov 15 15:55:56 2019 +0100
@@ -473,15 +473,16 @@
 
 
 JRT_ENTRY(void, Runtime1::store_flattened_array(JavaThread* thread, valueArrayOopDesc* array, int index, oopDesc* value))
-  assert(ArrayKlass::cast(array->klass())->storage_properties().is_flattened() || (ArrayKlass::cast(array->klass())->storage_properties().is_null_free() && value == NULL), "should not be called");
   if (ArrayKlass::cast(array->klass())->storage_properties().is_flattened()) {
     profile_flat_array(thread);
   }
 
   NOT_PRODUCT(_store_flattened_array_slowcase_cnt++;)
   if (value == NULL) {
+    assert(ArrayKlass::cast(array->klass())->storage_properties().is_flattened() || ArrayKlass::cast(array->klass())->storage_properties().is_null_free(), "should not be called");
     SharedRuntime::throw_and_post_jvmti_exception(thread, vmSymbols::java_lang_NullPointerException());
   } else {
+    assert(ArrayKlass::cast(array->klass())->storage_properties().is_flattened(), "should not be called");
     array->value_copy_to_index(value, index);
   }
 JRT_END


> - ciObjArrayKlass.cpp: why are these checks required?

A non null free value array has a subtype, the null free value array,
right?
Without that change, the c1 code assumes that if it sees any value
array, it knows the exact type.

> - TestLWorld.java:2318, can this be removed?
> - ValueTypeTest.java, nice refactoring. I had something similar in the queue. Can lines 553, 565 be
> removed?

Yep. Sorry, I forgot about those. 

> - Should the C1 and interpreter profiling be guarded by EnableValhalla?

The problem is that I replaced the existing aastore profiling. So with
-EnableValhalla we would go back to the previous aastore profiling and
previous MonomorphicArrayCheck?

> I've executed some testing, all passed! How did you verify performance?

Easy. I didn't. It obviously improves performance! :-)

> Please file an enhancement for this so we can better keep track of related tasks.

What you're saying is I need a bug id for this change, right?

Roland.