GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — src: update compile cache storage structure by joyeecheung · Pull Request #54291 · nodejs/node

Conversation

joyeecheung commented Aug 9, 2024 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Member

This refactors the compile cache handler in preparation for the JS API, and updates the compile cache storage structure into:

- $NODE_COMPILE_CACHE_DIR
  - $NODE_VERION-$ARCH-$CACHE_DATA_VERSION_TAG-$UID
      - $FILENAME_AND_MODULE_TYPE_HASH.cache

This also adds a magic number to the beginning of the cache
files for verification, and returns the status, compile
cache directory and/or error message of enabling the
compile cache in a structure, which can be converted as
JS counterparts by the upcoming JS API.

Refs: #53639

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

bricss, H4ad, and MomenNano reacted with rocket emoji

All reactions

🚀 3 reactions

joyeecheung added the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label

Aug 9, 2024

joyeecheung marked this pull request as ready for review

August 9, 2024 19:30

nodejs-github-bot added c++

Issues and PRs that require attention from people who are familiar with C++.

needs-ci PRs that need a full CI run. labels

Aug 9, 2024

github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label

Aug 9, 2024

nodejs-github-bot commented Aug 9, 2024

Copy link

Copy Markdown

Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/60994/

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

codecov Bot commented Aug 9, 2024 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Codecov Report

Attention: Patch coverage is 80.59701% with 13 lines in your changes missing coverage. Please review.

Project coverage is 87.08%. Comparing base (3cbeed8) to head (8f9eb07).
Report is 449 commits behind head on main.

Files with missing lines	Patch %	Lines
src/compile_cache.cc	81.81%	7 Missing and 1 partial ⚠️
src/env.cc	81.81%	2 Missing and 2 partials ⚠️
src/compile_cache.h	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #54291      +/-   ##
==========================================
- Coverage   87.11%   87.08%   -0.03%     
==========================================
  Files         647      647              
  Lines      181773   181965     +192     
  Branches    34889    34909      +20     
==========================================
+ Hits       158347   158470     +123     
- Misses      16739    16777      +38     
- Partials     6687     6718      +31

Files with missing lines	Coverage Δ
src/env.h	`97.87% <ø> (ø)`
src/compile_cache.h	`75.00% <0.00%> (-25.00%)`	⬇️
src/env.cc	`85.24% <81.81%> (-0.33%)`	⬇️
src/compile_cache.cc	`80.00% <81.81%> (-2.18%)`	⬇️

... and 43 files with indirect coverage changes

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

joyeecheung commented Aug 9, 2024

Copy link

Copy Markdown

Member Author

cc @nodejs/startup @nodejs/cpp-reviewers

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

addaleax approved these changes

Aug 10, 2024

View reviewed changes

anonrig reviewed

Aug 11, 2024

View reviewed changes

anonrig left a comment

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

We need more comments in this PR. It's extremely hard to keep track of the changes.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

src/compile_cache.cc Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread

src/compile_cache.h Resolved

Uh oh!

There was an error while loading. Please reload this page.

jasnell approved these changes

Aug 11, 2024

View reviewed changes


          src: update compile cache storage structure

8f9eb07

This refactors the compile cache handler in preparation for the
JS API, and updates the compile cache storage structure into:

- $NODE_COMPILE_CACHE_DIR
  - $NODE_VERION-$ARCH-$CACHE_DATA_VERSION_TAG-$UID
    - $FILENAME_AND_MODULE_TYPE_HASH.cache

This also adds a magic number to the beginning of the cache
files for verification, and returns the status, compile
cache directory and/or error message of enabling the
compile cache in a structure, which can be converted as
JS counterparts by the upcoming JS API.

joyeecheung force-pushed the refactor-compile-cache branch from afb5bbc to 8f9eb07 Compare

August 12, 2024 16:00

joyeecheung commented Aug 12, 2024

Copy link

Copy Markdown

Member Author

We need more comments in this PR. It's extremely hard to keep track of the changes.

FYI most of the changes here are preparation for https://github.com/joyeecheung/node/tree/compile-cache-api so I only sketched out what the changes are for in the commit messages, not in the comments.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Ethan-Arrowood approved these changes

Aug 12, 2024

View reviewed changes

joyeecheung added the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label

Aug 12, 2024

github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label

There was an error while loading. Please reload this page.

joyeecheung added the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label

Aug 15, 2024

github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label

Aug 15, 2024

nodejs-github-bot commented Aug 15, 2024

Copy link

Copy Markdown

Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/61133/

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

legendecas approved these changes

Aug 15, 2024

View reviewed changes

joyeecheung added the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label

Aug 16, 2024

github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label

Aug 16, 2024

nodejs-github-bot commented Aug 16, 2024

Copy link

Copy Markdown

Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/61163/

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

joyeecheung added the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label

Aug 18, 2024

github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label

Aug 18, 2024

nodejs-github-bot commented Aug 18, 2024

Copy link

Copy Markdown

Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/61204/

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

nodejs-github-bot commented Aug 18, 2024

Copy link

Copy Markdown

Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/61211/

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

joyeecheung added the commit-queue PRs queued for automated landing through the Commit Queue. label

Aug 19, 2024

nodejs-github-bot removed the commit-queue PRs queued for automated landing through the Commit Queue. label

Aug 19, 2024

nodejs-github-bot merged commit b246f22 into nodejs:main

Aug 19, 2024

nodejs-github-bot commented Aug 19, 2024

Copy link

Copy Markdown

Collaborator

Landed in b246f22

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

RafaelGSS pushed a commit that referenced this pull request

Aug 19, 2024


          src: update compile cache storage structure

1e3f772

This refactors the compile cache handler in preparation for the
JS API, and updates the compile cache storage structure into:

- $NODE_COMPILE_CACHE_DIR
  - $NODE_VERION-$ARCH-$CACHE_DATA_VERSION_TAG-$UID
    - $FILENAME_AND_MODULE_TYPE_HASH.cache

This also adds a magic number to the beginning of the cache
files for verification, and returns the status, compile
cache directory and/or error message of enabling the
compile cache in a structure, which can be converted as
JS counterparts by the upcoming JS API.

PR-URL: #54291
Refs: #53639
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Ethan Arrowood <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>

RafaelGSS mentioned this pull request

Aug 19, 2024

v22.7.0 proposal #54452

Merged

RafaelGSS pushed a commit that referenced this pull request

Aug 21, 2024


          src: update compile cache storage structure

e077ff1

This refactors the compile cache handler in preparation for the
JS API, and updates the compile cache storage structure into:

- $NODE_COMPILE_CACHE_DIR
  - $NODE_VERION-$ARCH-$CACHE_DATA_VERSION_TAG-$UID
    - $FILENAME_AND_MODULE_TYPE_HASH.cache

This also adds a magic number to the beginning of the cache
files for verification, and returns the status, compile
cache directory and/or error message of enabling the
compile cache in a structure, which can be converted as
JS counterparts by the upcoming JS API.

PR-URL: #54291
Refs: #53639
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Ethan Arrowood <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>

targos added the dont-land-on-v20.x label

Sep 21, 2024

Reviewers

anonrig

anonrig left review comments

jasnell

jasnell approved these changes

addaleax

addaleax approved these changes

legendecas

legendecas approved these changes

Ethan-Arrowood

Ethan-Arrowood approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

c++

Issues and PRs that require attention from people who are familiar with C++.

needs-ci

PRs that need a full CI run.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

8 participants

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

GeezerVison

Conversation

joyeecheung commented Aug 9, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nodejs-github-bot commented Aug 9, 2024

Uh oh!

codecov Bot commented Aug 9, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

joyeecheung commented Aug 9, 2024

Uh oh!

anonrig left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

joyeecheung commented Aug 12, 2024

Uh oh!

nodejs-github-bot commented Aug 12, 2024

Uh oh!

nodejs-github-bot commented Aug 14, 2024

Uh oh!

nodejs-github-bot commented Aug 14, 2024

Uh oh!

nodejs-github-bot commented Aug 15, 2024

Uh oh!

nodejs-github-bot commented Aug 15, 2024

Uh oh!

nodejs-github-bot commented Aug 15, 2024

Uh oh!

nodejs-github-bot commented Aug 16, 2024

Uh oh!

nodejs-github-bot commented Aug 18, 2024

Uh oh!

nodejs-github-bot commented Aug 18, 2024

Uh oh!

nodejs-github-bot commented Aug 19, 2024

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

joyeecheung commented Aug 9, 2024 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

codecov Bot commented Aug 9, 2024 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.