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

builder: align file dump order with prefetch list, fix #1488 #1492

Merged
merged 1 commit into from
Nov 27, 2023

Conversation

hangvane
Copy link
Contributor

@hangvane hangvane commented Nov 22, 2023

Relevant Issue (if applicable)

If there are Issues related to this PullRequest, please list it.

Fix #1488

Details

Please describe the details of PullRequest.

  1. The dump order for prefetch files does not match the order specified in prefetch list, so let's fix it, by recording the index of matched prefetch pattern, and then sort the prefetch files by indexes before dump.

Master:
image

Patch:
image

  1. The construction of Prefetch is slow due to inefficient matching of prefetch patterns.
    For the build of patterns, generate_patterns(), the check whether current pattern is existed or covered by existing patterns in prefetch.patterns, is changed from iterating every pattern to check whether the parent path have been recorded.
    For the build of prefetch.files, the rootfs tree is iterated in BFS only once in build_rafs(), and then cached in Prefetch to aviod duplicate BFS iteration in layout_blob_simple().

  2. Unit tests for prefetch are added.

Thus the build time prefetching is accelerated. For the nydusify convert time of a single layer Wordpress image with a total of 14,916 files, out of which 3,205 are prefetching files,

Master:
DirectoryToRafs: 19.13s
TarToRafs: 17.43s

Patch:
DirectoryToRafs: 7.08s
TarToRafs: 5.33s

Types of changes

What types of changes does your PullRequest introduce? Put an x in all the boxes that apply:

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation Update (if none of the other choices apply)

Checklist

Go over all the following points, and put an x in all the boxes that apply.

  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.



1. The dump order for prefetch files does not match the order specified in prefetch list,
so let's fix it.
2. The construction of `Prefetch` is slow due to inefficient matching of prefetch patterns,
By adopting a more efficient data structure, this process has been accelerated.
3. Unit tests for prefetch are added.

Signed-off-by: Wenhao Ren <wenhaoren@mail.dlut.edu.cn>
@hangvane hangvane requested a review from a team as a code owner November 22, 2023 12:49
@hangvane hangvane requested review from jiangliu, changweige and adamqqqplay and removed request for a team November 22, 2023 12:49
Copy link

codecov bot commented Nov 22, 2023

Codecov Report

Merging #1492 (2379ec4) into master (e2b131e) will increase coverage by 0.20%.
The diff coverage is 94.70%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1492      +/-   ##
==========================================
+ Coverage   62.50%   62.70%   +0.20%     
==========================================
  Files         123      123              
  Lines       43148    43248     +100     
  Branches    43148    43248     +100     
==========================================
+ Hits        26970    27119     +149     
+ Misses      14870    14818      -52     
- Partials     1308     1311       +3     
Files Coverage Δ
builder/src/core/bootstrap.rs 77.33% <100.00%> (-0.16%) ⬇️
builder/src/stargz.rs 74.67% <100.00%> (ø)
builder/src/tarball.rs 62.84% <100.00%> (ø)
builder/src/core/blob.rs 41.33% <0.00%> (-0.07%) ⬇️
builder/src/core/layout.rs 92.10% <96.55%> (+92.10%) ⬆️
builder/src/directory.rs 0.00% <0.00%> (ø)
builder/src/core/prefetch.rs 74.70% <95.68%> (+28.41%) ⬆️

... and 1 file with indirect coverage changes

@jiangliu
Copy link
Collaborator

LGTM, thanks for the great work:)

@jiangliu jiangliu merged commit 71dfc6f into dragonflyoss:master Nov 27, 2023
26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Builder: file dump order does not match prefetch list
2 participants