Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Minimise the scope of the <VM> generic type #127

Open
qinsoon opened this issue Sep 2, 2020 · 1 comment
Open

Minimise the scope of the <VM> generic type #127

qinsoon opened this issue Sep 2, 2020 · 1 comment
Labels
A-general Area: all code base (issues with this label may be divided into more concrete issues) C-cleanup Category: Cleanup P-normal Priority: Normal.

Comments

@qinsoon
Copy link
Member

qinsoon commented Sep 2, 2020

At the moment, most of the types has a <VM> generic type to it, even if the type does not actually use <VM> (but some of its functions uses it). We should do a cleanup to remove the generic type where possible. These are a few points raised from our discussion:

  • If a function in the type uses <VM>, we can make the function generic rather than make the type generic.
  • If possible, only types that are logically used by the VM should have the type parameters, such as MMTK<VM>, Mutator<VM>. Types such as Collector<VM>, Space<VM> do not sound logically right.
  • Types without <VM> can be unit tested without mocking.
  • If a call to <VM> is not performance critical, it is possible for us to use dyn VM rather than generic type. However this requires some possibly API-breaking changes, as currently we never instantiate a VM object (all functions in VMBinding are static functions).
@qinsoon qinsoon added C-cleanup Category: Cleanup A-general Area: all code base (issues with this label may be divided into more concrete issues) labels Sep 2, 2020
@qinsoon qinsoon added this to the v0.1 milestone Sep 2, 2020
@qinsoon qinsoon modified the milestones: v0.1, v0.2 Nov 3, 2020
@qinsoon qinsoon removed this from the v0.2 milestone Dec 18, 2020
@qinsoon qinsoon added the P-normal Priority: Normal. label Jan 8, 2024
@wks
Copy link
Collaborator

wks commented Jan 8, 2024

We discussed ProcessEdgesWork in today's meeting. There is a related issue for that #599, but the main problem with ProcessEdgesWork is that it contains too much information about a trace. It contains more than just <VM>, but also the concrete work packet types for the current trace.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-general Area: all code base (issues with this label may be divided into more concrete issues) C-cleanup Category: Cleanup P-normal Priority: Normal.
Projects
None yet
Development

No branches or pull requests

2 participants