Skip to content

Commit

Permalink
Apply suggestions from code review
Browse files Browse the repository at this point in the history
Co-authored-by: Hugo van Kemenade <[email protected]>
  • Loading branch information
savannahostrowski and hugovk authored Jan 27, 2025
1 parent eb03372 commit 19cad22
Showing 1 changed file with 10 additions and 8 deletions.
18 changes: 10 additions & 8 deletions peps/pep-0774.rst
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,8 @@ requirement for JIT builds in preparation for shipping the JIT off by default in
3.14. The consensus at the sprint was that it would be sufficient to provide
pre-generated stencils for non-debug builds for Tier 1 platforms and that
checking these files into the CPython repo would be adequate for the limited
number of platforms (though more options have been explored; see Rejected
Ideas).
number of platforms (though more options have been explored; see `Rejected
Ideas <PEP 774 Rejected Ideas_>`__).

Currently, building CPython with `the JIT requires LLVM
<https://github.com/python/cpython/tree/main/Tools/jit#installing-llvm>`__ as a
Expand Down Expand Up @@ -82,9 +82,9 @@ This approach:
However, this approach does result in a slight increase in overall
repository size. Comparing repo growth on commits over the past 90 days, the
difference between the actual commits and the same commits with stencils added
amounts to a difference of 0.03MB per stencil file. This is a small increase in
the context of the overall repository size, which has grown by 2.55MB in the
same time period. For six stencil files, this amounts to an upper bound of 0.18MB.
amounts to a difference of 0.03 MB per stencil file. This is a small increase in
the context of the overall repository size, which has grown by 2.55 MB in the
same time period. For six stencil files, this amounts to an upper bound of 0.18 MB.

These stencils could become larger in the future with changes to register
allocation, which would introduce 5-6 variants per instruction in each stencil
Expand Down Expand Up @@ -149,7 +149,7 @@ usual.
However, as part of this, we will introduce a new step that diffs the current
stencils in the repo against those generated in CI. If there is a diff for a
platform’s stencil file, a patch file for the updated stencil is generated and
the step will fail. Each patches is uploaded to GitHub Actions. After CI is
the step will fail. Each patch is uploaded to GitHub Actions. After CI is
finished running across all platforms, the patches are aggregated into a single
patch file for convenience. You can download this aggregated patch, apply it
locally, and commit the updated stencils back to your branch. Then, the
Expand Down Expand Up @@ -179,6 +179,8 @@ Ignoring the stencils themselves and any necessary JIT README changes, the
changes to the source code to support reproducible stencil generation and
hosting are minimal (around 150 lines of changes).

.. _PEP 774 Rejected Ideas:

Rejected Ideas
==============

Expand Down Expand Up @@ -229,9 +231,9 @@ workflows. Also, depending on the provider, this type of hosting comes with
additional cost, which we’d like to avoid.

Using Git LFS
--------------
-------------

Git LFS adds tool dependency for contributors, complicating the development
Git Large File Storage (LFS) adds a tool dependency for contributors, complicating the development
workflow, especially for those who may not already use Git LFS. Git LFS does not
work well with offline workflows since files managed by LFS require an internet
connection to fetch when checking out specific commits, which is disruptive for
Expand Down

0 comments on commit 19cad22

Please sign in to comment.