Re: Rust frontend patches v3

2022-10-28 Thread Arthur Cohen

Hi David,

On 10/26/22 23:15, David Malcolm wrote:

On Wed, 2022-10-26 at 10:17 +0200, arthur.co...@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 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.



Hope this is constructive
Dave



Thanks a lot for the input,

All the best,

Arthur






OpenPGP_0x1B3465B044AD9C65.asc
Description: OpenPGP public key


OpenPGP_signature
Description: OpenPGP digital signature
-- 
Gcc-rust mailing list
Gcc-rust@gcc.gnu.org
https://gcc.gnu.org/mailman/listinfo/gcc-rust


Re: Rust frontend patches v3

2022-10-28 Thread Richard Biener via Gcc-rust
On Fri, Oct 28, 2022 at 1:45 PM Arthur Cohen  wrote:
>
> Hi David,
>
> On 10/26/22 23:15, David Malcolm wrote:
> > On Wed, 2022-10-26 at 10:17 +0200, arthur.co...@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 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.

collection points are explicit, but some might be hidden behind
middle-end APIs, in particular once you call cgraph::finalize_compilation_unit
you should probably expect collection.

Richard.

> > Hope this is constructive
> > Dave
> >
>
> Thanks a lot for the input,
>
> All the best,
>
> Arthur
>
>
>
>
-- 
Gcc-rust mailing list
Gcc-rust@gcc.gnu.org
https://gcc.gnu.org/mailman/listinfo/gcc-rust


Re: Rust frontend patches v3

2022-10-28 Thread David Malcolm via Gcc-rust
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.co...@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 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.

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
> 
> 
> 
> 

-- 
Gcc-rust mailing list
Gcc-rust@gcc.gnu.org
https://gcc.gnu.org/mailman/listinfo/gcc-rust


Re: Rust frontend patches v3

2022-10-28 Thread Arthur Cohen



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.co...@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 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








OpenPGP_0x1B3465B044AD9C65.asc
Description: OpenPGP public key


OpenPGP_signature
Description: OpenPGP digital signature
-- 
Gcc-rust mailing list
Gcc-rust@gcc.gnu.org
https://gcc.gnu.org/mailman/listinfo/gcc-rust


Re: Rust frontend patches v3

2022-10-28 Thread David Malcolm via Gcc-rust
On Fri, 2022-10-28 at 17:20 +0200, Arthur Cohen wrote:
> 
> 
> 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.co...@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...]

> > 
> > 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.

Great!   Though as noted, for libgccjit it slows the testsuite down
*massively*, so you might want to bear that in mind.  I'm doing it for
libgccjit because libgccjit looks like a "frontend" to the rest of the
GCC codebase, but it's a deeply weird one, and so tends to uncover
weird issues :-/

Dave

> 
> 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
> > > 
> > > 
> > > 
> > > 
> > 

-- 
Gcc-rust mailing list
Gcc-rust@gcc.gnu.org
https://gcc.gnu.org/mailman/listinfo/gcc-rust