(draft) MMTk Enhancement Proposal (MEP)

[wks]

MMTk Enhancement Proposal (MEP)

This is a draft. We decided to try this process for three months, and review whether this process is good enough or needs changing.

This is a meta-issue that describes the format and process of MEP.

TODO: After trial, we should put a formal version of this document in mmtk-core/docs.

An MMTk Enhancement Proposal (MEP) is a more formal variant of issue. It has a special format, and will undergo a more thorough review process. Its goal is helping the MMTk developers making more informed decisions.

MEP is inspired by the Java Enhancement Proposal, described in https://openjdk.org/jeps/1 However, unlike JEP which is more open-ended, MEP is more focused on making design decisions.

When is MEP required?

An MEP is required when making a significant change to the design of the MMTk core. It is applicable to any kind of significant changes, including but not limited to:

• Bug fixes, including performance bug fixes, that require change to a major part of the MMTk core.
• Changes to the MMTk core to implement a feature demanded by bindings.
• Major refactoring to the MMTk core.

One purpose of MEP is reducing the risks to the future development of MMTk. Large-scale changes and public API changes usually indicate such risks, but these are only indicators, not criteria. The assesment of risks is subjective, and we need to discuss in order to reach consensus.

Note: JEP is also required for things that "require two or more weeks of engineering effort" and/or "are in high demand by developers or customers". We don't judge whether we need MEP based on engineering effort or public demand. Many PRs for MMTk require multiple weeks of work and rigorous testing, but most of them can be settled with regular issues and PRs. We label priorities using GitHub issue tags, such as P-normal, P-high, etc. If a feature is requested often, and we have man power for that, we can raise the priority.

Format

A MEP will be posted as a GitHub issue in the mmtk-core repository. It should contain certain tags:

• The MEP tag, used to identify MEPs.
• Area (A-*), for example, A-gc-algorithm.
• Category (C-*), for example, C-refactoring. Note that not all MEP are "enhancement" in the sense of C-enhancement. Some MEPs may simply be intended for fixing long-standing hard-to-fix bugs by making non-trivial changes.
• Goal (G-*), for example, G-safety.

We use the format of JEP (https://openjdk.org/jeps/2) as a frame of reference, but deviate from it when needed.

A MEP should have the following sections:

• TL;DR (summary)
• Goal
• Non-goal (optional)
• Success Metric (optional)
• Motivation
• Description
  • Impact on Performance
  • Impact on Future Performance Improvement
  • Impact on Public API
  • Impact on Software Engineering
• Testing (optional)
• Alternatives (optional)
• Risks and Assumptions (optional)
• Related Issues (optional)

Note: Sections in JEP but not in MEP:

• Dependencies: We have the Related Issues section, instead.

Sections

TL;DR

This section should use about one to three sentences to summarize the MEP. JEP calls it "Summary", but we call it "TL;DR" (too long, didn't read) to emphasize that it should be short enough so that readers (including those in a hurry) can get the main idea very quickly without reading through the MEP.

Goal

What are the goals of the proposal? Preferrably enumerate the goals in a list.

Non-Goals

Optional. Use this section to explicitly exclusive goals out of the scope of the current MEP.

Success Metric

Optional. What profits this MEP will bring after it is implemented? This can incldue improvements in performance, safety, readability, maintainability, etc. Enumerate the profits in a list, but describe the details in the Description section.

Motivation

Why should this work be done? Who is asking for it?

Make sure the readers understand the problem this MEP is trying to address. You can also mention the languages, VMs, or users that need this enhancement.

If there are alternative ways to solve the problem, you can mention them here, but keep in mind that there is an Alternatives section for adding more details.

Description

This is the main section of the MEP. Describe the enhancement in detail.

If you have already made prototype implementations for this MEP, add hyperlinks to the relevant PRs, commits, repos, etc.

If not, describe how you intend to implement this MEP. You should think about all parts of mmtk-core that your MEP may interact with.

This section should include several subsections:

• Impact on Performance
• Impact on Future Performance Improvement
• Impact on Public API
• Impact on Software Engineering

Impact on Performance

Describe how this MEP will affect the performance. "This MEP should not have any impact on performance" is still a valid description if it is so.

Impact on Future Performance Improvement

Describe whether this MEP will bring any opportunity or difficulty for improving the performance of MMTk in the future.

The immediate performance impact is important, but a more important thing is the long-term opportunity to improve performance in the future (See the next subsection). It is OK for us to accept temporary minor performance reduction to make more significant improvements possible. On the contrary, it is bad to make changes to temporarily improve performance and make long-term imporovements hard or impossible.

Impact on Public API

Describe how this MEP will change the public API. "This MEP makes no change to the public API" is still a valid description if it is so.

Impact on Software Engineering

Describe whether this MEP will make software engineering easier or more difficult. Will it make the code easier or harder to understand, maintain and/or change?

One goal of MMTk is making GC development easy. If a developer wants to develop, for example, a new GC algorithm, it should be easy to implement it quickly and easily in MMTk. We generally don't want changes that make this difficult.

Testing

Optional. If applicable, describe the way to reproduce the problem, and the way to check if the change actually works. For MMTk, it includes but is not limitied to what (micro or macro) benchmarks to use, and which VM binding (with or without changes) to use.

Alternatives

Optional. If there are more than one way to solve the problem, describe them here.

Risks and Assumptions

Optional. For the design changes of MMTk, this part mainly includes assumptions about, for example, the VM's requirements, the GC algorithms we are supporting, the OS/architecture MMTK is running on, etc. If those assumptions no longer hold, we may need to reconsider the design. Describe such concerns in this section.

Related Issues

Optional. If there are related issues, PRs, etc., include them here.

Sometimes people create ordinary issues and PRs to fix some problems, and later MMTk developers consider that the change is too fundamental and needs a more thorough reviewing process. If that is the case, add hyperlinks to those original issues and PRs.

Reviewing Process

TODO

[qinsoon]

After reading the issue, I think there are a few things that are worth some discussion.

• Name. Have we agreed on the name MEP? I noticed that in the issue, @wks tried to clarify the difference between MEP and the C-enhancement Category: Enhancement label we currently have. We could avoid using the word enhancement in MEP, or rename the label. As stated in the issue, "An MEP is required when making a significant change to the design of the MMTk core", we may call it MDP (MMTK Design Proposal) to emphasise its focus on the design part of MMTk.
• MEP criteria and review. Currently, the distinction between what constitutes a MEP and a standard PR/issue is too ambiguous. We should establish more concrete criteria for MEPs, and these criteria should align with the review process we choose to implement. If we opt for a rigorous review process, the criteria should be set such that it limits the number of MEPs. On the other hand, if we decide on a more lightweight review process, it would be appropriate to expect a higher number of MEPs.
• Doc. I think in the end, we should have a markdown file that describes MEP in /docs, and we should be able to close this issue.
• Issue template. We should set up an issue template for MEP (https://github.com/mmtk/mmtk-core/issues/templates/edit). Possibly we should also set up issue templates for other purposes (e.g. bug report, feature request, etc).

[wks]

After reading the issue, I think there are a few things that are worth some discussion.

* Name. Have we agreed on the name MEP? I noticed that in the issue, @wks tried to clarify the difference between MEP and the 
  [
    C-enhancement](https://github.com/mmtk/mmtk-core/labels/C-enhancement)
    Category: Enhancement
   label we currently have. We could avoid using the word enhancement in MEP, or rename the label. As stated in the issue, "An MEP is required when making a significant change to the design of the MMTk core", we may call it `MDP` (MMTK Design Proposal) to emphasise its focus on the design part of MMTk.

Sorry for not making it clear. This issue is a draft, and the name "MEP" is only a proposal. I am open to other names if "MEP" is not appropriate. "MDP" is also reasonable, given that we mainly focus on design changes.

But it is also ambiguous what is a "design". For example, #1004 (It is where we first had the idea of a more rigorous reviewing process.) It changes the way how plans and spaces are instantiated. We may consider it a design change, but from the VM developers' point of view, it is an implementation detail inside mmtk-core; and from the GC developers' point of view, it doesn't change the relationship between plans and spaces, and it doesn't even affect any concrete GC algorithms. So I think we can use MEP as long as the change is significant.

* MEP criteria and review. Currently, the distinction between what constitutes a MEP and a standard PR/issue is too ambiguous. We should establish more concrete criteria for MEPs, and these criteria should align with the review process we choose to implement. If we opt for a rigorous review process, the criteria should be set such that it limits the number of MEPs. On the other hand, if we decide on a more lightweight review process, it would be appropriate to expect a higher number of MEPs.

Yes. It will be helpful to establish more concrete criteria, in the form of "a significant change that requires a MEP is usually characterized as ...", but there is no hard borderline between MEP and a standard PR/issue. In practice, I expect some MEPs will be "upgraded" from ordinary PRs and issues simply because people think they are too complicated.

And I don't think the number of MEPs matter because having too many fundamental changes doesn't mean we can just let some of them get merged without undergoing discussion. We may instead design a process that mitigates the risk of making significant changes, and make MEPs easier to review. For example,

• We may tentatively approve an MEP, but require the feature to be guarded by a Cargo feature, so that it can undergo more testing before being fully accepted (or reverted).
• We may require a prototype implementation and performance evaluation before discussing the MEP in group, so that when discussing, it is easier for us to know whether an MEP is a good idea or not.

* Doc. I think in the end, we should have a markdown file that describes MEP in `/docs`, and we should be able to close this issue.

👍

* Issue template. We should set up an issue template for MEP (https://github.com/mmtk/mmtk-core/issues/templates/edit). Possibly we should also set up issue templates for other purposes (e.g. bug report, feature request, etc).

👍

[qinsoon]

These are what we discussed in today's meeting:

• MEPs could be bug fixes, performance bug fixes, mmtk-core changes demanded by bindings, major refactoring, etc.
• MEPs are intended for code/design changes that may have profound impact for the future. It is about predicting what would happen in the future, and the prediction could be very subjective.
• The review process for MEP could be heavy weight. It may take prolonged discussion of weeks. Thus we need to limit the number of MEPs.
• The MMTk team need to decide on two things, based on consensus:
  1. whether a PR/issue should be an MEP. One member escalates the issue, and discusses with the team. It should not take long to decide if an issue should be an MEP.
  2. review the MEP. This may take long.
• I suggested if we should have some hard criteria for MEP exemption. For example, if a PR does not change public interface, and does not change internal interface (which depends on how we define as 'internal interface'), it is not an MEP. Steve suggested it should be an indicator rather than criteria.
• Zixian suggested that a slow and anit-productive MEP process (for both reviewers and submitters) may make it harder for people to upstream code. We currently have a lot of outstanding changes that should be upstreamed and haven't been upstreamed. MEP may make the situation worse, and make people tend to keep their changes in their own forks.

[qinsoon]

Based on the discussion, these are my proposed review process.

Initiate an MEP

An MEP is initiated by creating an MEP issue with the format drafted by OP.

Escalate normal PRs/issues and request for MEP

For normal PRs/issues, if any team member thinks it should be an MEP, they should escalate it and discuss with the team. If the team decide that it should be an MEP, the PR/issue should be set to 'Request for MEP', and expects an MEP issue from the contributor. If there is no further action from the contributor for a period of time, the PR/issue is considered as stale and it may get assigned to an MMTk team member or may be closed.

Note that a team member may escalate an PR/issue for normal discussion, rather than requesting for MEP. As we discussed, MEP is a heavy-weight process, and we should not abuse requesting it.

Review an MEP

1. Criteria

The MMTk team will first decide if the proposal meets the criteria for being an MEP. If it does not meet the criteria, the proposal issue will be closed, and related changes should be treated as a normal PR/issue.

The criteria for what need to be an MEP is mostly subjective, based on a consensus model within the MMTk team. We also provide a list of exemption for what do not need to be an MEP.

MEP Exemption

MEP is intended to help avoid design changes that may have profound negative impact in the future. Some changes will not have profound impact, and can be easily reverted if necessary. They should be exempt from the heavy-weight MEP process, and should not be escalated to request for MEP. The exemption is intended to ensure that we won't abuse using MEP and that we won't impose burden on the contributors to submit an extra MEP proposal. An exempted PR may still be escalated for team discussion, but it is exempt from being requested for MEP (submitting a MEP proposal, and going through the MEP process).

Exemption 1: Well-encapsulated changes

Changes that are well-encapsulated and decoupled intrinsically can be easily corrected in the future and will not have profound impact for the future. A PR that has no public API change, and no module API change between the top-level modules (plan, policy, scheduler, util and vm at the time of writing) is exempt from MEP.

2. Review

The MMTk team will discuss the proposal in weekly meetings. This process may take a while. We will keep posting the discussion to the MEP issue, and encourage further inputs from the contributor, and the community. An MEP may get updated and refined during the process.

3. Outcome

At the end of the review, the MEP will be accepted or rejected based on the consensus of the MMTk team. If an MEP is accepted, A PR may follow the MEP and will be reviewed with the normal PR review process.

If an MEP is rejected, future related MEPs may not be reviewed again unless they are substantially different. We encourage people to get involved in the review discussion, and refine the proposal so it will be accepted.

[steveblackburn]

Proposed edits to @wks's template

--

TL;DR

This section should use about one to three sentences to summarize the MEP. JEP calls it "Summary", but we call it "TL;DR" (too long, didn't read) to emphasize that it should be short enough so that readers (including those in a hurry) can get the main idea very quickly without reading through the MEP.

Goals

What are the goals of the proposal? This should be succinct. If there's more than one goal, enumerate them in a list.

Non-Goals

Optional. Use this section to explicitly exclusive goals out of the scope of the current MEP.

Success Metric

Optional. How do we determine whether the MEP is a success? This can include improvements in performance, safety, readability, maintainability, etc. Enumerate the success metrics in a list (details should be in the Description section).

Motivation

Why should this work be done? Who is asking for it?

Make sure the readers understand the problem this MEP is trying to address. You can also mention the languages, VMs, or users that need this enhancement.

If there are alternative ways to solve the problem, you can mention them here, but keep in mind that there is an Alternatives section for adding more details.

Risks

Outline the long-term risks posed by this MEP and how those risks are mitigated. The core purpose of the MEP process is to avoid the unintentional introduction of changes that bring long-term negative impacts to MMTk. This section is about identifying and accounting for risks associated with such negative outcomes.

Long Term Software Engineering Risks

Enumerate possible negative long-term software engineering impacts of this MEP, and for each explain how that risk will be mitigated.

One of the core goals of MMTk is making GC development easy. If a developer wants to develop, for example, a new GC algorithm, it should be easy to implement it quickly and easily in MMTk. We don't want changes that make this difficult.

Long Term Performance Risks

Enumerate possible negative long-term performance impacts of this MEP, and for each explain how that risk will be mitigated. Note: this is not about the immediate performance impact of the MEP, but about the impact on future work. For example, this includes identifying changes that may impede the future introduction of entirely new algorithms, or entirely new optimizations.

It is OK for us to accept temporary minor performance reduction to make more significant improvements possible. On the contrary, it is bad to make changes to temporarily improve performance and make long-term imporovements hard or impossible.

Impact on API

Outline how this MEP will affect APIs, both public and internal. Enumerate the cases, and for each case, explain how the risk of negative consequences will be mitigated and/or justify the change.

Description

This is the main section of the MEP. Describe the enhancement in detail.

If you have already made prototype implementations for this MEP, add hyperlinks to the relevant PRs, commits, repos, etc.

If not, describe how you intend to implement this MEP. You should think about all parts of mmtk-core that your MEP may interact with.

This section should include several subsections:

• Impact on Performance
• Impact on Future Performance Improvement
• Impact on Public API
• Impact on Software Engineering

Impact on Performance

Describe how this MEP will affect the performance. "This MEP should not have any impact on performance" is still a valid description if it is so.

Impact on Future Performance Improvement

Describe whether this MEP will bring any opportunity or difficulty for improving the performance of MMTk in the future.

The immediate performance impact is important, but a more important thing is the long-term opportunity to improve performance in the future (See the next subsection). It is OK for us to accept temporary minor performance reduction to make more significant improvements possible. On the contrary, it is bad to make changes to temporarily improve performance and make long-term imporovements hard or impossible.

Impact on Public API

Describe how this MEP will change the public API. "This MEP makes no change to the public API" is still a valid description if it is so.

Impact on Software Engineering

Describe whether this MEP will make software engineering easier or more difficult. Will it make the code easier or harder to understand, maintain and/or change?

One goal of MMTk is making GC development easy. If a developer wants to develop, for example, a new GC algorithm, it should be easy to implement it quickly and easily in MMTk. We generally don't want changes that make this difficult.

Testing

Optional. If applicable, describe the way to reproduce the problem, and the way to check if the change actually works. For MMTk, it includes but is not limitied to what (micro or macro) benchmarks to use, and which VM binding (with or without changes) to use.

Alternatives

Optional. If there are more than one way to solve the problem, describe them here and explain why this approach is preferred.

Risks and Assumptions

Optional. For the design changes of MMTk, this part mainly includes assumptions about, for example, the VM's requirements, the GC algorithms we are supporting, the OS/architecture MMTK is running on, etc. If those assumptions no longer hold, we may need to reconsider the design. Describe such concerns in this section.

Related Issues

Optional. If there are related issues, PRs, etc., include them here.

Sometimes people create ordinary issues and PRs to fix some problems, and later MMTk developers consider that the change is too fundamental and needs a more thorough reviewing process. If that is the case, add hyperlinks to those original issues and PRs.

[qinsoon]

#1056 introduced a document for MEP: https://github.com/mmtk/mmtk-core/blob/master/docs/contribute/mep.md, and we start using the MEP procedure.

We should either close this issue or make it clear what else needs to be done for the issue.

[wks]

Yes. Since the MEP process document has been added (#1056), we can close this issue.

#1056 introduced a document for MEP: https://github.com/mmtk/mmtk-core/blob/master/docs/contribute/mep.md, and we start using the MEP procedure.

We should either close this issue or make it clear what else needs to be done for the issue.