mmtk / mmtk-core

Memory Management ToolKit
https://www.mmtk.io
Other
379 stars 69 forks source link

Add back `DummyVM` as a part of the porting guide. Minor changes to MMTk initialization in the porting guide. #1142

Closed qinsoon closed 5 months ago

qinsoon commented 5 months ago

Following the discussion here: https://mmtk.zulipchat.com/#narrow/stream/315620-Porting/topic/Porting.20MMTK.20to.20Clasp.20Common.20Lisp/near/442123897. It is useful for the language implementers to have a Rust binding crate that implements all the boilerplate code and can compile to start with. This PR adds back DummyVM to the porting guide, and includes some minor changes to the porting guide.

wks commented 5 months ago

We shouldn't bury the dummyvm directory too deep into the docs directory. MMTk core developers will need to keep maintaining it as we change the API of mmtk-core. It is better to move it to a more accessible location. I suggest we place it in mmtk-core/dummyvm, directly. We may add a README.md in mmtk-core/dummyvm to state its purpose and link to the porting guide.

qinsoon commented 5 months ago

We shouldn't bury the dummyvm directory too deep into the docs directory. MMTk core developers will need to keep maintaining it as we change the API of mmtk-core. It is better to move it to a more accessible location. I suggest we place it in mmtk-core/dummyvm, directly. We may add a README.md in mmtk-core/dummyvm to state its purpose and link to the porting guide.

I put it as a part of the porting guide, as I feel it is only relevant for people who start a new binding. I don't think it is important and relevant enough to be at the top level of the project root directory. It was placed in vmbindings/dummyvm, because it was introduced when we had other bindings in the core repo such as vmbindings/jikesrvm and vmbindings/openjdk.

For maintainability, I modified the CI scripts to build and style-check DummyVM in the CI (in the same way as we check the mygc in the tutorial). We will know if it no longer compiles. Also I changed the DummyVM a bit to have more methods as unimplemented!(), as we no longer need it to run with the default implementation. We just need it to compile so people can have a reasonable start point for porting.

If you do think we should put it as a place that is easier for people to browse to from the root directory, I think we can put it in mmtk-core/docs/dummyvm.

wks commented 5 months ago

If you do think we should put it as a place that is easier for people to browse to from the root directory, I think we can put it in mmtk-core/docs/dummyvm.

Yes. mmtk-core/docs/dummyvm is good enough, at least shorter than the previous mmtk-core/vmbindings/dummyvm. We can add a sentence in its own README.md which says it is probably only relevant for people who start a new binding.

And we can remove mmtk-core/docs/header given that we acknowledge that dummyvm is part of the documentation. There is another mmtk.h in docs/header, and we can remove that in favor for the one in dummyvm.

qinsoon commented 5 months ago

The 3 optional checks failed, but they are expected: