Cleanup/document JIT arraylet enablement API

[0xdaryl]

Questions around this confusing and poorly documented API come up every so often and we are left to return to first principles to figure out what the semantics are. Another question was raised recently.

Let's use this issue to get the semantics straight and either document or alter the code to make this clear.

I sanitized some of the questions and replies from internal Slack discussions on this API for this issue so they don't get lost.

Developer 0

I think I found an odd bug re arraylets…?

bool OMR::Compilation::generateArraylets()
   {
   if (TR::Compiler->om.canGenerateArraylets())
      {
      return TR::Compiler->om.useHybridArraylets() ? false : true;
      }
   else
      {
      return false;
      }
   }

In OMR ObjectModel:

   bool canGenerateArraylets() { return false; }
   bool useHybridArraylets() { return false; }

In OpenJ9 ObjectModel:

   bool canGenerateArraylets() { return usesDiscontiguousArraylets(); }
   bool useHybridArraylets() { return usesDiscontiguousArraylets(); }

Regardless if usesDiscontiguousArraylets will return true or false, given that the two methods canGenerateArraylets and useHybridArraylets will always return the same answer, generateArraylets() will always return false. Or am I missing something?

Developer 1

Can anybody explain the intention of OMR::Compilation::generateArraylets(), please? If I understand it correctly, it never returns true.

The function depends on canGenerateArraylets() and useHybridArraylets() in ObjectModel, and both of them are defined by usesDiscontiguousArraylets() in OpenJ9. [1][2]

• When usesDiscontiguousArraylets() is false, canGenerateArraylets() is also false, and generateArraylets() simply returns false.
• When usesDiscontiguousArraylets() is true, generateArraylets() checks useHybridArraylets() after canGenerateArraylets(). Both of them are true, and generateArraylets() returns false as the result.
  So generateArraylets() always returns false, regardless of the value from usesDiscontiguousArraylets().

Do I overlook anything?

[1] https://github.com/eclipse/omr/blob/92b485082a75459b1ccbc209ab6360f31b04d4f8/compiler/compile/OMRCompilation.cpp#L1662-L1672
[2] https://github.com/eclipse/openj9/blob/61c3a700129045c0cc7819942a0a0c7412c14682/runtime/compiler/env/J9ObjectModel.hpp#L92-L93

Replies

Daryl

One question to simplify all of this is, are there any build specs or GC policies that don’t use hybrid arraylets? I remember years and years ago that the realtime policy did not, but balanced did. Is that still true?

Aleks

Balanced can use hybrid. Can't remember for Metronome ATM.

Developer 2

Recently, I was investigating the method generateArraylets() and noticed something odd. The method checks to see if TR::Compiler->om.canGenerateArraylets() is true and follows it up with a check to TR::Compiler->om.useHybridArraylets(). If the latter returns false, generateArraylets() returns true and vice versa.
This is a strange because both useHybridArraylets() and canGenerateArraylets() have the same method body, and therefore would return identical results. This leads me to believe that generateArraylets() never returns true. I find this a bit surprising because a significant amount of code is guarded by calls to generateArraylets() which may be missing out on generating.
To make absolutely sure, I replaced the return true branch on generateArraylets() with a fatal assert and kick started a personal build, which reported no failures, confirming that the method seems to be returning false at all times.
My question is if I’m missing something obvious here? Is there a reason we overlap API calls and miss out on generating code guarded by generateArraylets()?

Replies

Developer 3

FYI as it seems optimizer and x86 codegen are the primary consumers of the generateArraylets() API. There are mixed uses on Power, and on Z we only seem to use the om.canGenerateArraylets() API. It is worrisome if we are not generating proper checks under balanced and the delta between gencon and balanced could be larger than we measure if this is fixed.

Developer 4

So the two checks are not identical in spirit from my understanding historically (I agree they have been fused now and we'd need to check with the GC team on what the status of the implementation is). Under arraylets, there is a question of what do you do with short arrays that would normally fit in the spine of the array

One option is to allocate an arraylet, put the data in the arraylet and away you go - it is just like any other discontriguous array. The other option is to put the data in the spine, unless the spine is too small, in which case you put the data in arraylets. But what do you with the leftover? e.g., if you don't have enough to fill the final arraylet. Again, you could put that data in the spine or you could allocate an arraylet for it. So we have traditionally had a mode where the spine will hold the data and if the data doesn't fit then we'll go to the arraylets and, I think but will need to check, that we would put the left over data from the part filled arraylet in the spine to save allocating another arraylet. But I believe the two queries did exist to determine which of those different versions of arraylets we were supporting - e.g., is the extra data in the spine or in a final arraylet?

Developer 3

The hybrid arraylets is what I think you may be referring to which are explained in [1]. I beleve they are slated for deprecation but haven't been yet? I'm not sure what the generateArraylets() API is supposed to model though.
[1] https://eclipse-omr.org/double-map-arraylets/

Developer 4

When we do enable double mapping there won’t be such thing as hybrid arraylets anymore. But balanced and metronome still make use of both hybrid and discontiguous arraylets which is the default behaviour. If an arraylet leftover data fits in the spine then the arraylet becomes hybrid. But if all data fits in spine then arraylet is contiguous. And the last layout is discontiguous where all arraylet data resides in their leaves.

Developer 3

In it’s current form, shouldn’t we expect generateArraylets() to return true for both hybrid and discontiguous though? Or in other words, anytime the array doesn’t have a contiguous layout.

Developer 4

Yes, I was about to say that, OMR::Compilation::generateArraylets() checks seems very odd since looking at it it is impossible to return true

Daryl

Was a final disposition reached on this discussion? I’m just trying to understand the semantics of these APIs and whether some Doxygen comments should be added to the code.

Developer ?

For me it seems that many if not all of the methods in compiler/env/OMRObjectModel.hpp are unimplemented. Which brings back to the behavior of OMR::Compilation::generateArraylets() always returning false.
The OMR version is overridden in OpenJ9 and it is there that it is hooked up - that is not unusual for the compiler component. So I think the historical intent of the APIs is as stated - generateArraylets means the arrays are not guaranteed to be contiguous while hybrid arraylets tells us whether data can be in the spine. Looking in history r14 says the APIs had been merged in their current form where they had been modified to return the same result. Given the difference in the double mapping model vs existing these differences may become more important again, but the API usages will need to be audited as part of any change to allow them to vary since the difference has probably not been well understood for some time and they have been synonyms for a number of years

Developer ?

When it comes to double mapping we don’t really have hybrid arraylets, but just discontiguous ones, hence the difference. And in the future, once the off-heap part is implemented these differences might become even more important like you mentioned

[0xdaryl]

FYI @knn-k @rmnattas @zl-wang @amicic

[rmnattas]

It seems to me that generateArraylets() is trying to wear multiple-hats:

• "generateArraylets means the arrays are not guaranteed to be contiguous". Needed for optimizations decisions at least.
• If hybrid arraylets are enabled "is the extra data in the spine or in a final arraylet". Should be a separate check.

Given that this is on the objectModel level not a specific array, determining between contiguous/discontiguous arraylets shouldn't matter as it's per array basis.

Then we have the "new" cases:

• Double-mapping (arraylets, contiguous elements, but discontiguous between array header and elements)
• Off-heap (no arraylets, contiguous elements, but discontiguous between array header and elements)

When saying contiguous and discontiguous there's two meaning with double-mapping or off-heap, which is also confusing in other areas of the codebase:

1. array elements are contiguous(gencon, off-heap) or not (discontiguous arraylets)
2. array header + array elements are contiguous (beside each other) in the heap (gencon, contiguous arraylets) or not (hybrid/discontiguous arraylets, off-heap)

[zl-wang]

Indeed, it is confusing, but we should aim to simplify and document it.

1. do we still contemplate anything further (and in the future) on double-mapping? if not, throw away all related to double-mapping;
   now, we are down to 2 policies with discontiguous arrays; such that
2. was hybridArrayLets query added to accommodate then-still-early double-mapping, given its returning the same answer always as generateArrayLets query currently? if yes, it can be eliminated.

[knn-k]

AArch64 codegen has the calls to all of the three APIs, generateArraylets(), canGenerateArraylets(), and useHybridArraylets(), like the p codegen.

In addition, J9VM_GC_ENABLE_DOUBLE_MAP is set as OFF in cmake/caches/linux_aarch64.cmake and osx_aarch64.cmake.

[rmnattas]

From what I can understand, given that the plan was that hybrid arraylets won't be a thing with double mapping, the check of hybrid arraylets is to check between "arraylets with double-mapping" or "arraylets without double-mapping".
Hence the hybrid arraylets check is not needed and can be removed, and if the double-mapping work continued in the future a check for it can be added.

Currently generateArraylets() and canGenerateArraylets() are used interchangeably in many places, in the JIT at least, for the same meaning (disabling transformations that can't be done on arraylets).

A comment states the intended difference:

   // This query answers whether or not this VM is capable of generating arraylet
   // trees.  This does not imply that it will for this compilation unit: you must ask
   // Compilation::generateArraylets() to answer that.
   //
   bool canGenerateArraylets() { return false; } // OMR implementation

I believe generateArraylets() should check canGenerateArraylets() then usesDiscontiguousArraylets() only. usesDiscontiguousArraylets() returns true if discontiguous arraylets (j9gc_modron_configuration_discontiguousArraylets) is set.

Although canGenerateArraylets() currently only checks usesDiscontiguousArraylets(), it can be modified in the future given the intention of it.

---

@amicic An upcoming off-heap commit will fix _usesDiscontiguousArraylets to be false for off-heap, even if j9gc_modron_configuration_discontiguousArraylets is set.
Is that the correct way or j9gc_modron_configuration_discontiguousArraylets shouldn't be set when off-heap is enabled?
rmnattas@0e3c109#diff-fabbcf44a3119ba7de74e09e6fa5ab9a23f12edb4b573f150f1c14c8dcb2b0fcR82-R94

[amicic]

Hi @rmnattas, your question is reasonable. I think it's ok to change the behaviour of what we return for j9gc_modron_configuration_discontiguousArraylets, when off-heap is enabled (although we could consider changing the name of the key, but keep the behaviour).

So, the new behaviour would be to return 1 if (arraylet leaf size is set && off heap disabled).

In that case you don't need to explicitly query j9gc_off_heap_allocation_enabled, and leave your code as-is.

We (@LinHu2016 or you) can do such a change right away (since currently off is disabled, it won't change the behaviour of what j9gc_modron_configuration_discontiguousArraylets returns).

As for some earlier terminology confusion, we introduced a new term/APIs in GC, so that an array instance could have:

• adjacent or non-adjacent data to header (non-adjacent only possible for offheap enabled)
• contiguous or discontiguous data (discontiguous only possible for legacy region based heaps, but not for the new offheap variant).
  You may want to use that new term (adjacent) in your code base/comments, too.