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

MMTk Enhancement Proposal #1056

Merged
merged 3 commits into from
Dec 22, 2023
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
265 changes: 265 additions & 0 deletions docs/team/mep.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,265 @@
MMTk Enhancement Proposal (MEP)
===============================

An MMTk Enhancement Proposal (MEP) is a more formal variant of issue. It has a special format, and
qinsoon marked this conversation as resolved.
Show resolved Hide resolved
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
qinsoon marked this conversation as resolved.
Show resolved Hide resolved
public API changes usually indicate such risks, but these are only indicators, not criteria. The
assessment of risks is subjective, and we need to discuss in order to reach consensus.
qinsoon marked this conversation as resolved.
Show resolved Hide resolved

Note: JEP is also required for things that "require two or more weeks of engineering effort" and/or
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would suggest not mentioning too much about JEP, especially when it is irrelevant. 1. As you said, MEP is clearly different from JEP. 2. Not many people are that familiar with JEP so it is hard for them to draw connection between the two.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree. I removed all places that mention JEP, keeping only one sentence that mentions that MEP is inspired by JEP.

"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
qinsoon marked this conversation as resolved.
Show resolved Hide resolved
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
qinsoon marked this conversation as resolved.
Show resolved Hide resolved
tags:

- **The `MEP` tag**
- It is 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:
qinsoon marked this conversation as resolved.
Show resolved Hide resolved

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

Note: Sections in JEP but not in MEP:
qinsoon marked this conversation as resolved.
Show resolved Hide resolved

- 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",
qinsoon marked this conversation as resolved.
Show resolved Hide resolved
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

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.

## 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, repositories, 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 will be long, and will usually be divided into many subsections. The following
subsections must be included:

- Impact on Performance
- 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 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?

## 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. It should include the following subsections:

- Long Term Performance Risks
- Long Term Software Engineering Risks
- Impact on API

### 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 improvements hard or impossible.

### 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.

### 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.

## Testing

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 limited 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.

## 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.

# MEP Reviewing Process

## Initiate an MEP

An MEP is initiated by creating an MEP issue with the format described in this document.

### 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

### 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.

### 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.

### 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.

## Communication

After an MEP is implemented, the MMTk team shall announce the significant changes as the results of
qinsoon marked this conversation as resolved.
Show resolved Hide resolved
the MEP to make them known to the community. We shall communicate with our sponsors about such
changes, too, in our regular meetings.

<!--
qinsoon marked this conversation as resolved.
Show resolved Hide resolved
vim: ts=4 sw=4 sts=4 et tw=100
-->
Loading