ammarhakim / gkyl

This is the main source repo for the Gkeyll 2.0 code. Please see gkeyll.rtfd.io for details.
https://gkeyll.readthedocs.io/en/latest/
62 stars 18 forks source link

Use Proto for Moments app #119

Closed liangwang0734 closed 1 year ago

liangwang0734 commented 1 year ago

Two changes:

The changes to allow attaching __gc is a Lua 5.1 hack (Luajit uses 5.1). Note that presently, other g2 codes that implement __gc all rely on a C type (through xsys or luajit metatype ), but the new changes to Proto work for pure Lua tables.

In principle, this should have no impact on all codes except Moments.lua since __gc is attached only when the class carries one, and no existing Proto children have __gc.

ammarhakim commented 1 year ago

@liangwang-phys can you put some print statements into the Proto code to see if the GC is actually being called. I remember trying this and there was some funky stuff and GC was never called explicitly. Do this in the unit tests for Proto. Once you determine that we can merge

ammarhakim commented 1 year ago

One other thing to do is see if the GC method is called in the Moment App. It should be simple to check.

liangwang0734 commented 1 year ago

Diff:

diff --git a/App/Moments/Moments.lua b/App/Moments/Moments.lua
index ba9ad3e4..5a9b993e 100644
--- a/App/Moments/Moments.lua
+++ b/App/Moments/Moments.lua
@@ -1149,7 +1149,9 @@ App.init = function(self, tbl)
 end

 function App:__gc()
+   print("calling __gc on", self.app)
    C.gkyl_moment_app_release(self.app)
+   print("calling __gc done")
 end
 App.apply_ic = function(self)
    C.gkyl_moment_app_apply_ic(self.app, self.t0)

Results:

Main loop took 1.97284 secs to complete
calling __gc on cdata<struct gkyl_moment_app *>: 0x55f715c93240
calling __gc done