-
Notifications
You must be signed in to change notification settings - Fork 722
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.
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
Update macros J9JAVAARRAYCONTIGUOUS_EA for off heap #20565
base: master
Are you sure you want to change the base?
Conversation
Update Macro J9JAVAARRAYCONTIGUOUS_EA and J9JAVAARRAYCONTIGUOUS_EA_VM to handle all the cases(IndexableDataAddrPresent/or not and VirtualLargeObjectHeapEnabled/or not). if IndexableDataAddrPresent if VirtualLargeObjectHeapEnabled J9JAVAARRAYCONTIGUOUS_WITH_DATAADDRESS_VIRTUALLARGEOBJECTHEAPENABLED else J9JAVAARRAYCONTIGUOUS_WITH_DATAADDRESS_VIRTUALLARGEOBJECTHEAPDISABLED else J9JAVAARRAYCONTIGUOUS_WITHOUT_DATAADDRESS Signed-off-by: lhu <[email protected]>
@amicic @dmitripivkine Could you please review the changes, Thanks |
we might not want to deliver this too early since it will cause small regression to standard GCs interpreter perf, due to extra is-present check |
? (&((elemType*)((((J9IndexableObjectContiguousCompressed *)(array)) + 1)))[index]) \ | ||
: (&((elemType*)((((J9IndexableObjectContiguousFull *)(array)) + 1)))[index])) | ||
|
||
#define J9JAVAARRAYCONTIGUOUS_WITHOUT_DATAADDRESS_EA_VM(javaVM, array, index, elemType) \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't like the fact that there is a reference to DATAADDRESS outside of appropriate build flag
and there are 2 issues:
- DATAADDRESS is outside of J9VM_ENV_DATA64
- the build flag should really be JVM_GC_SPARSE_HEAP_ALLOCATION (not J9VM_ENV_DATA64)
I'll think if we can reshuffle cleanly
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
perhpas the simplest is to use BASE (or some other generic term) rather than WITHOUT_DATAADDRESS
@dmitripivkine, opinion?
for using the proper build flag, as we discussed recently, let's don't change it yet #20503 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, we can use BASE
Update Macro J9JAVAARRAYCONTIGUOUS_EA and J9JAVAARRAYCONTIGUOUS_EA_VM to handle all the cases(IndexableDataAddrPresent/or not and VirtualLargeObjectHeapEnabled/or not).
if IndexableDataAddrPresent
if VirtualLargeObjectHeapEnabled
J9JAVAARRAYCONTIGUOUS_WITH_DATAADDRESS_VIRTUALLARGEOBJECTHEAPENABLED
else
J9JAVAARRAYCONTIGUOUS_WITH_DATAADDRESS_VIRTUALLARGEOBJECTHEAPDISABLED
else
J9JAVAARRAYCONTIGUOUS_WITHOUT_DATAADDRESS