Re: Rust frontend patches v3

[Arthur Cohen <arthur.cohen@embecosm.com>]

[-- Attachment #1.1.1: Type: text/plain, Size: 3624 bytes --]



On 10/28/22 15:06, David Malcolm wrote:
> On Fri, 2022-10-28 at 13:48 +0200, Arthur Cohen wrote:
>> Hi David,
>>
>> On 10/26/22 23:15, David Malcolm wrote:
>>> On Wed, 2022-10-26 at 10:17 +0200, arthur.cohen@embecosm.com wrote:
>>>> This is the fixed version of our previous patch set for gccrs -
>>>> We've
>>>> adressed
>>>> the comments raised in our previous emails.
>>>
>>> [...snip...]
>>>
>>> (Caveat: I'm not a global reviewer)
>>>
>>> Sorry if this is answered in the docs in the patch kit, but a high-
>>> level question: what's the interaction between gccrs and gcc's
>>> garbage
>>> collector?  Are the only GC-managed objects (such as trees) either
>>> (a)
>>> created near the end of the gccrs, or (b) common globals created at
>>> initialization and with GTY roots?
>>
>> We only create trees at the last point of our compilation pipeline,
>> before directly writing them to the backend. This then calls a
>> `write_global_definitions` method, that we ported over directly from
>> the
>> Go frontend. Among other things, this method has the role of
>> preserving
>> trees from the GC using `go_preserve_from_gc()` (or
>> `rust_preserve_from_gc()` in our case).
>>
>> Elsewhere in our pipeline, we never call any garbage-collection
>> routines
>> or GC-related functions.
>>
>>> Are there any points where a collection happen within gccrs?  Or is
>>> almost everything stored using
>>> gccrs's own data structures, and are these managed in the regular
>>> (non-
>>> GC) heap?
>>
>> This is correct. We have an AST representation, implemented using
>> unique
>> pointers, which is then lowered to an HIR, also using unique
>> pointers.
>>
>>> I skimmed the patches and see that gccrs uses e.g. std::vector,
>>> std::unique_ptr, std::map, and std::string; this seems reasonable
>>> to
>>> me, but it got me thinking about memory management strategies.
>>>
>>> I see various std::map<T, tree> e.g. in Rust::Compile::Context; so
>>> e.g.
>>> is the GC guaranteed never to collect whilst this is live?
>>
>> This is a really interesting question, and I hope the answer is yes!
>> But
>> I'm unsure as to how to enforce that, as I am not too familiar with
>> the
>> GCC GC. I'm hoping someone else will weigh in. As I said, we do not
>> do
>> anything particular with the GC during the execution of our
>> `CompileCrate` visitor, so hopefully it shouldn't run.
> 
> I'm guessing that almost all of gccrs testing so far has been on
> relatively small examples, so that even if the GC considers collecting,
> the memory usage might not have exceeded the threshold for actually
> doing the mark-and-sweep collection, and so no collection has been
> happening during your testing.
> 
> In case you haven't tried yet, you might want to try adding:
>    --param=ggc-min-expand=0 --param=ggc-min-heapsize=0
> which IIRC forces the GC to actually do its mark-and-sweep collection
> at every potential point where it might collect.

That's very helpful, thanks a lot. I've ran our testsuite with these and 
found no issues, but we might consider adding that to our CI setup to 
make sure.

Kindly,

Arthur

> I use these params in libgccjit's test suite; it massively slows things
> down, but it makes any GC misuse crash immediately even on minimal test
> cases, rather than hiding problems until you have a big (and thus
> nasty) test case.
> 
> Hope this is helpful
> Dave
> 
> 
>>
>>> Hope this is constructive
>>> Dave
>>>
>>
>> Thanks a lot for the input,
>>
>> All the best,
>>
>> Arthur
>>
>>
>>
>>
> 

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3195 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]