iets3.opensource icon indicating copy to clipboard operation
iets3.opensource copied to clipboard

Introduce interpreter, trace & test infrastructure interfaces

Open enikao opened this issue 1 year ago • 1 comments

When using the KernelF interpreter in customer projects, we often run into issues where too much infrastructure is hard-coded, preventing us from adjusting it to our needs. Specifically, I'd like to introduce some kind of weak references at several places, so processing big models doesn't run out of memory because of too many interpreter trace objects in memory.

This PR includes:

  • Changing all relevant behavior methods to virtual
  • Introducing interfaces for interpreter and tracing-related classes, and use them throughout.

So far, the changes should be binary compatible to previous versions. This would break once we changed the return types from e.g. ValueAndTrace to IValueAndTrace. The migration should be straight-forward: 1/ swapping the two types; 2/ changing field access of ValueAndTrace.value to IValueAndTrace.getValue()

We also identified a potential bug in DefaultCoverageAnalyzer.isInInterestingContextEx() (see comment).

enikao avatar Oct 09 '24 19:10 enikao

@enikao is master really the correct target branch?

mgronover avatar Oct 10 '24 12:10 mgronover

@enikao is master really the correct target branch?

I am targeting MPS 2023.2, and I think that's master branch? Or should I target it at an older branch and migrate forward?

enikao avatar Oct 13 '24 09:10 enikao

The changes look good to me. The bug you found is a real bug! Please fix it (exchange n by it ).

@AlexeiQ , @arimer for me it would be good to have these changes also in earlier version, what do you think?

mgronover avatar Oct 14 '24 08:10 mgronover

I just pushed lots of tests, and a fix for the bug.

enikao avatar Oct 22 '24 21:10 enikao

After merging in master, I have a dead reference in http://127.0.0.1:63320/node?ref=r%3A9741ad23-5b7e-4040-b070-51e24fd6bfee%28org.iets3.core.expr.repl.editor%29%2F6371013116349124293

image

enikao avatar Oct 23 '24 08:10 enikao

@enikao try a ./gradlew --refresh-dependencies it might be the case the the platform has changed but you are still using an older cached version of it.

arimer avatar Oct 23 '24 08:10 arimer

@enikao try a ./gradlew --refresh-dependencies it might be the case the the platform has changed but you are still using an older cached version of it.

That helped, thanks!

enikao avatar Oct 23 '24 09:10 enikao

@enikao I triggered your build manually on TC, cause your branch name was not matching the build trigger config. Branches should start with: grafik

arimer avatar Oct 23 '24 11:10 arimer

Is there anything I can help with for review or merging?

enikao avatar Oct 24 '24 11:10 enikao

Superseded by #1098

enikao avatar Oct 30 '24 08:10 enikao