[foreign-abi] [Rev 01] RFR: 8245988: Add a special VaList carrier

[Jorn Vernee]

On Fri, 29 May 2020 15:05:37 GMT, Jorn Vernee <jvernee at openjdk.org> wrote:

>> Great work.
>> 
>> Didn't read through all the changes - but I have a question on the API - why did you opted for a separate Reader
>> abstraction? Seems like VaList is already stateful anyway (since it can be closed), why not dropping all the reader
>> methods directly into VaList?  Also (but can be done later) - we need va_copy too.
>
> Reader is an attempt to allow users to re-read the same VaList. It also gives an oppertunity to declare how many
> arguments the VaList is expected to hold (something we don't know when constructing the VaList instance). I think
> moving the reader methods to VaList makes sense as well.   I'll look into implementing a `copy()` method as well.
> 
> As discussed offline, I'll also look into adding methods to convert back and forth between a MemoryAddress from a
> VaList. As well as adding a benchmark that compares VaList with a manually specialized varargs down call (using `...`).

Updated to address review comments:
- Merged Reader and VaList.
- Added methods to convert between a VaList and a MemoryAddress directly.
- Let SysVVaList use the underlying va_list's memory instead of have separate fields (this makes copying and converting
  to MemoryAddress easier/possible).
- Added javadoc to VaList and VaList.Builder (forgot to do this before).
- Added a copy() method for copying a VaList, plus some accompanying tests.
- Adding missing tests for the skip method, and fixed a bug I found with that.
- Added a benchmark that compares passing a VaList with using a specilalized varargs invocation that creates a va_list on
  the native side instead.

wrt the benchmark; the VaList case is currently 15x slower on Windows :( I have to do more investigation to figure out
exactly why, but my guess is that this is mostly due to the need for native heap allocations when we create the VaList
in Java, while the native implementation can allocate on the stack. Off course, call intrinsification will probably
also have a big effect here.

Still, VaList provides easier access to native functions accepting a va_list, and for upcalls, makes passing a variable
number of arguments possible in the first place.

-------------

PR: https://git.openjdk.java.net/panama-foreign/pull/184