(draft) MMTk Enhancement Proposal (MEP) #1041
Comments
|
After reading the issue, I think there are a few things that are worth some discussion.
|
wks
changed the title
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.
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,
👍
👍 |
|
These are what we discussed in today's meeting:
|
|
Based on the discussion, these are my proposed review process. Initiate an MEPAn MEP is initiated by creating an MEP issue with the format drafted by OP. Escalate normal PRs/issues and request for MEPFor 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 MEP1. CriteriaThe 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 ExemptionMEP 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 changesChanges 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 ( 2. ReviewThe 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. OutcomeAt 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. |
|
Proposed edits to @wks's template -- TL;DRThis 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. GoalsWhat are the goals of the proposal? This should be succinct. If there's more than one goal, enumerate them in a list. Non-GoalsOptional. Use this section to explicitly exclusive goals out of the scope of the current MEP. Success Metric
MotivationWhy 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. RisksOutline 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 RisksEnumerate 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 RisksEnumerate 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 APIOutline 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. DescriptionThis 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 PerformanceDescribe 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.
|
|
#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. |
|
Yes. Since the MEP process document has been added (#1056), we can close this issue.
|
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:
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-corerepository. It should contain certain tags:MEPtag, used to identify MEPs.A-*), for example,A-gc-algorithm.C-*), for example,C-refactoring. Note that not all MEP are "enhancement" in the sense ofC-enhancement. Some MEPs may simply be intended for fixing long-standing hard-to-fix bugs by making non-trivial changes.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:
Note: Sections in JEP but not in MEP:
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
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
The text was updated successfully, but these errors were encountered: