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.

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

WIP:p: NULL initialize dataAddr field for 0 size arrays #21011

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

VermaSh
Copy link
Contributor

@VermaSh VermaSh commented Jan 24, 2025

Update array inline allocation sequence to initialize dataAddr field
only for non-zero size arrays. Field should be left blank for zero
size arrays.

@VermaSh VermaSh force-pushed the inline_array_allocation_p branch from 135b379 to b2c903c Compare January 24, 2025 17:11
Update array inline allocation sequence to initialize dataAddr field
only for non-zero size arrays. Field should be left blank for zero
size arrays.

Signed-off-by: Shubham Verma <[email protected]>
@VermaSh VermaSh force-pushed the inline_array_allocation_p branch from b2c903c to 37f2833 Compare January 24, 2025 18:55
@VermaSh VermaSh changed the title WIP: NULL intialize dataAddr field for 0 size arrays on p WIP:p: NULL initialize dataAddr field for 0 size arrays Jan 24, 2025
@VermaSh
Copy link
Contributor Author

VermaSh commented Jan 24, 2025

@zl-wang / @rmnattas here are my changes for inline allocation sequence on power. Marked it as WIP because I still need to investigate the failures.

@VermaSh VermaSh marked this pull request as draft January 24, 2025 18:58
@rmnattas
Copy link
Contributor

I'm not sure if I'm understanding what's needed for this correctly but my understanding as bases of my following comments:
1- J9IndexableObjectDisontiguous is for arraylets mainly and not used for OffHeap, hence we only care about contiguous object shapes(?)
2- needZeroInit being false means that dataAddrField is assumed 0 already.

Listed the different cases the code handles to make sure each works correctly and comments are related to:
1- Places that use the disontiguous object shape dataAddrFieldOffset or headerSize which will be wrong in compressed ref (and confusing although correct in full ref)
2- Cases where needZeroInit is false but code writes obj+headerSize into dataAddrField even if array len is 0.

3 blocks
a) line 6519
b) line 6554
c) line 6564

8 cases
1) compressed & isVariableLen & needZeroInit
2) compressed & isVariableLen & !needZeroInit
3) compressed & !isVariableLen & needZeroInit
4) compressed & !isVariableLen & !needZeroInit
5) !compressed & isVariableLen & needZeroInit
6) !compressed & isVariableLen & !needZeroInit
7) !compressed & !isVariableLen & needZeroInit
8) !compressed & !isVariableLen & !needZeroInit


1,2 -> a: needZeroInit uses OffsetOfDiscontiguousDataAddrField? !needZeroInit uses OffsetOfContiguousDataAddrField correctly.
3 -> if-const-0 {b: use discontiguous offset?} else {c: uses contiguous offset}
4 -> c: needZeroInit is false but you always write to dataAddrField. If len is 0, you write obj+headerSize over the already 0 dataAddrField, right?
5,6 -> c: doesn't do the runtime check if len is 0, always set dataAddr = obj+headerSize
7 -> if-const-0 {b: use discontiguous offset?} else {c: uses contiguous offset}
8 -> c: doesn't do the runtime check if len is 0, always set dataAddr = obj+headerSize

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants