Switch to CBinding.jl v1.0 or Clang.jl

[ranocha]

Our current setup using CBindingGen will not work on Julia v1.7, see #37. Thus, we need to switch to CBinding.jl v1.0 or Clang.jl. However, neither option (CBinding.jl or Clang.jl) is perfect right now, as seen in #38 and discussed on Slack. Here is a summary of the current state.

Clang.jl

The new generator interface of Clang.jl looks promising. It basically creates Julia code to use the standard Julia interface for calling C code. However, it has some limitations such as

• no support of va_list arguments
• there is no REPL tab completion for properties defined automatically (REPL autocompletion JuliaInterop/Clang.jl#316)
• we don't get docstrings by default but there might be ways to achieve this, see Function and other signatures in comments JuliaInterop/Clang.jl#195

CBinding.jl v1.0

CBinding.jl provides another way of wrapping C code but doesn't use the Julia base C interface. This can make it more difficult to understand what's going on (e.g., trixi-framework/Trixi.jl#637 (comment)) but it might also be more intuitive sometimes. However, it doesn't work right now as we would like, see #38...

Patterns currently in use in Trixi.jl based on CBindingGen v0.9

Here is a list of the access patterns that we use in Trixi.jl at the moment.

• p4est.connectivity
• conn.num_trees
• conn.num_vertices
• unsafe_wrap(Array, conn.vertices, (3, n_vertices))
• unsafe_wrap(Array, conn.tree_to_vertex, (2^NDIMS, n_trees))
• Ptr{Int}(quadrant.p.user_data)
• tree = unsafe_load_tree(info.p4est, info.treeid + 1)
• tree.quadrants_offset
• quad_id = offset + info.quadid
• Ptr{Int}(info.quad.p.user_data)
• quadrants = unsafe_wrap_sc(p4est_quadrant_t, trees[tree].quadrants)
• quad.x, quad.y, quad.z
• side.is_hanging
• local_quad_id = side.is.full.quadid
• info.sides.elem_count
• sides[1].treeid
• trees[1].quadrants_offset

[sloede]

As far as I can tell, the missing support for va_list is the only "hard" technical reason not to use Clang.jl. However, switching to Clang.jl will require some overhaul of the Trixi code, and it would make using nested C data structures more inconvenient.

CBindingGen.jl + CBinding.jl v0.9 so far has been a "it just works" kind of solution, and it is very convenient to use, even though its harder (much harder IMHO) to understand what's going on under the hood. However, with the new approach of CBinding.jl v1.0 unifying both CBinding.jl and CBindingGen.jl, pre-generating bindings does not work as easily anymore, and especially the fine-tuning of the generation process has been lost.

In an attempt to summarize the current state: Clang.jl is lacking some features that prevent it from being a drop-in replacement, i.e., it is too bare-bones. CBinding.jl v1.0 seems like overkill for what we try to do while missing some finer-grained control, i.e., it is too high level.

[krrutkow]

This can make it more difficult to understand what's going on...

I admit it's not particularly easy to understand, but CBinding v1 should be easier to understand than pre-v1 versions that relied on a lot of hacky workarounds and less elegant mechanisms.

...pre-generating bindings does not work as easily anymore...

What is the reason for using pre-generated bindings?

...and especially the fine-tuning of the generation process has been lost.

What would you like to tune about the generation process? (The process is much more robust now, so incorporating a workaround hook hasn't been necessary yet for any of my use-cases.)

However, it doesn't work right now as we would like, see #38...

Was this regarding the Carray function argument bug?

Thanks!

[sloede]

...pre-generating bindings does not work as easily anymore...

What is the reason for using pre-generated bindings?

Some C header files required by p4est do not seem to be installed by default on all supported operating systems. Specifically, we had issues on macOS without an Xcode installation, and I believe we also had issues on Windows. Therefore we decided to add pre-generated bindings for the P4est_jll-based use of P4est.jl, since this is what 99% of at least our current users are using anyways.

...and especially the fine-tuning of the generation process has been lost.

What would you like to tune about the generation process? (The process is much more robust now, so incorporating a workaround hook hasn't been necessary yet for any of my use-cases.)

Specifically, we would like to exclude the macro hacks sc_extern_c_hack_* as did for CBindingGen.jl:

P4est.jl/deps/build.jl

Line 197 in dce3abe

startswith(name, "sc_extern_c_hack_") && return false

Further, there were some warnings about failed binding generation that we would like to get rid of, e.g., by excluding problematic but non-essential symbols (see also the first ~20 lines of the snippet posted in https://github.com/trixi-framework/P4est.jl/blob/update-to-cbinding-v1/mwe.md#mwe-1).

However, it doesn't work right now as we would like, see #38...

Was this regarding the Carray function argument bug?

No, that was regarding the fact that we got all these warnings above, together with the failure to manage to store and load pre-generated bindings. Essentially, that's what we tried in MWE2, but I failed to understand what's wrong. Quoting from https://github.com/trixi-framework/P4est.jl/blob/update-to-cbinding-v1/mwe.md#mwe-2:

ERROR: LoadError: LoadError: syntax: "$" expression outside quote around /mnt/ssd/home/mschlott/hackathon/P4est.jl/libp4est-mwe2.jl:10087

Essentially, the main issue for me was the lack of first-class support for storing the pre-generated bindings in a fashion similar to how CBindingGen.jl operated before. I wanted to discuss this further internally before considering to pull you in as well, but now you're already here 😄, thus...

Would you be willing to help us with setting up our CBinding.jl@v1.0 setup such that we can continue using it in P4est.jl, specifically to get pregeneration working again? I saw that you already set up a working version in an own branch (https://github.com/krrutkow/P4est.jl/tree/update-to-cbinding-v1), which seems to be working for the non-pregenerated case. If you're up for it, we could turn this into a proper PR and take it from there.

[krrutkow]

Sure, I can help with the transition to v1. Is the deps/build.jl known to be working on the various problematic platforms you mentioned? I can use that as a starting point if it is. The downstream usage of the bindings will probably have changes as well (since the converted types are now all immutable to improve performance), but I can support that too. Is running tests a simple way to hit everything that is effected?

[sloede]

Sure, I can help with the transition to v1.

This is great to hear, thanks a lot in advance!

Is the deps/build.jl known to be working on the various problematic platforms you mentioned? I can use that as a starting point if it is.

Yes, it was what we've been using so far with CBindingGen.jl and CBinding.jl v0.9. One thing that is not so easy to test this are the pre-generated bindings - they are stored in a separate repo and downloaded as artifacts. However, during development one could store the pregenerated bindings just inside the branch and only remove it before the merge.

The downstream usage of the bindings will probably have changes as well (since the converted types are now all immutable to improve performance), but I can support that too. Is running tests a simple way to hit everything that is effected?

Unfortunately, we do not have a lot of useful tests set up so far (see also #26). Thus I'd say no, just running the tests is not a very good validation :-/

However, what would be a good first test would be to get the small example from MWE 1 up and running with a pre-generated bindings file. That is, essentially getting MWE 2 to work by taking your successful attempt from your branch but storing the pre-generated output and includeing it appropriately.

I think if you could demonstrate us how to achieve this for a similar MWE with P4est_jll.jl, we can take it from there and put it in deps/build.jl ourselves. So far, I have struggled (and failed!) to use @macroexpand and include to make pregeneration work.

[krrutkow]

@sloede Here are a couple branches for pre-generated bindings and runtime-generated bindings (the preferred method):

• https://github.com/krrutkow/P4est.jl/tree/cbinding-v1-pregen
• https://github.com/krrutkow/P4est.jl/tree/cbinding-v1

The pre-generated approach doesn't support conversion of macros and documentation because Julia doesn't yet have full support for serializing the AST back into parseable code. The runtime-generated bindings include those items, and can also support converting the inline functions defined in the headers as well (but that is still a more experimental feature).

The generate function used in the build script is actually something that will be included in a future version of CBinding (since it is the generalized form of what happens in the CBinding generate.jl script). I haven't tested the pre-generated bindings across different platforms, but the generate function should have made all the path references relative in the bindings.

I tried to make everything minimal so it is easier to see how the two approaches are nearly identical. And separating libsc and libp4est bindings is relatively easy (like #34) to do as well.

[sloede]

@krrutkow Wow, this was extremely fast! Thank you very much for your support! I have looked at your approach, and while I cannot claim I understand everything that's going on in the generate function, it seems quite straightforward to actually use the bindings this way. A possible way forward would be to move the generate function to our "preprocessing" stage and decide during the package build whether to just load the bindings from a file or whether to generate the bindings on the fly, which, as you stated, is the preferred version.

I am out of office for the next two weeks, but afterwards I will try to test out your solution with some practical tests, including whether it works with Trixi.jl. Again, thanks a lot for your effort!

[sloede]

@krrutkow Thanks again for your interest and help in getting P4est.jl set up with CBinding v1. I have started testing with https://github.com/krrutkow/P4est.jl/tree/cbinding-v1-pregen. There are a two questions that have come up so far:

1. Is there a functional/performance difference between accessing a[].b vs. a.b[]? For example, following the examples in https://github.com/trixi-framework/P4est.jl#usage, I tried

   julia> using P4est
   
   julia> conn_ptr = p4est_connectivity_new_periodic()
   CBinding.Cptr{var"c\"struct p4est_connectivity\""}(0x0000000005b15de0)
   
   julia> conn_ptr[].num_trees
   1
   
   julia> conn_ptr.num_trees[]
   1

   Is one preferable over the other? Intuition tells me that the former is probably slower since it has to dereference/load an entire struct first, then access its integer field, while the latter can probably directly load the value from C-managed memory. But I wanted to be sure and ask you :-)
2. How to support macro definitions like

   #define P4EST_MAXLEVEL 30

   I understand that it's currently not possible to serialize macro definitions to a file, but in the meantime, what would be a canonical way to support them already without having a breaking change once macro definitions work? I thought about defining P4EST_MAXLEVEL = 30 in src/P4est.jl, but this might be bad if macros will ultimately be accessed with [] as well.
   I also noted that specifically P4EST_MAXLEVEL does not seem to work while other macros exist. This applies also to the cbinding-v1 branch from above - any idea why this doesn't work even though in that branch you only use the ji options?

[krrutkow]

1. I have not actually observed a difference in performance or in the generated assembly code, so compiler optimization seems to make them equivalent. However, there could be a difference in performance due to loading a whole structure vs. a single field, as you noticed, so I tend to dereference at the end (x.y.z[]) rather than in the middle (x.y[].z) which is also easier on the eyes I think.
2. Sorry, I don't describe it very well (or at all) in the README, but C macros are converted to a Julia macro and then seamlessly hooked in with the c"..." string macro. C macros are by far the worst to deal with, and because sometimes C macros are used to "override" types/variables/functions, it is only possible to support them generally/automatically through the use of Julia macros.

julia> using P4est, CBinding

julia> @P4EST_MAXLEVEL
30

julia> c"P4EST_MAXLEVEL"
30

julia> P4EST_MAXLEVEL
ERROR: UndefVarError: P4EST_MAXLEVEL not defined

[sloede]

I have not actually observed a difference in performance or in the generated assembly code, so compiler optimization seems to make them equivalent. However, there could be a difference in performance due to loading a whole structure vs. a single field, as you noticed, so I tend to dereference at the end (x.y.z[]) rather than in the middle (x.y[].z) which is also easier on the eyes I think.

OK, thanks for the clarification. I agree on the aesthetic appeal issue; I also think it makes it easier to decide where to put the [] for deeply nested structures (always at the end, duh!) :-)

Sorry, I don't describe it very well (or at all) in the README, but C macros are converted to a Julia macro and then seamlessly hooked in with the c"..." string macro. C macros are by far the worst to deal with, and because sometimes C macros are used to "override" types/variables/functions, it is only possible to support them generally/automatically through the use of Julia macros.

Again, thanks for the clarification! That means if I want to be able to expose the same API no matter whether pre-generated or ad-hoc generated bindings are used, I should probably implement something like

macro P4EST_MAXLEVEL()
  :(30)
end

as long as pre-generated bindings are used that do not support macro generation.

I will keep trying to use Trixi with the branch of yours to see if more stuff pops up.

[sloede]

Alright, so I managed to work around the issue of missing macros but now I am getting other errors that I don't understand that seem to originate from differences in how CBinding v0.9 and v1.0 create bindings.

@krrutkow Would you mind actually opening PRs for your two branches against P4est.jl and allow maintainer edits? That way your contributions will be recorded appropriately but it would allow me to try to figure out some stuff (and ask for more help 😬) by directly pushing to your branch. Otherwise, I can also just create a fork of P4est.jl myself and - with your permission - copy over your branches.

[krrutkow]

Done! #42

[sloede]

Great, thanks! Could you please also open a PR for https://github.com/krrutkow/P4est.jl/tree/cbinding-v1? I think to debug using P4est.jl with Trixi.jl might be easier if we do not have the issue of working with the pre-generated bindings on top of everything else. My thinking was to use https://github.com/krrutkow/P4est.jl/tree/cbinding-v1 to figure out what needs to change for existing users of P4est.jl, document & fix it, and once it works figure out how to make it run with pre-generated bindings.