Skip to content

Binary cache: async push_success #908

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

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
69 commits
Select commit Hold shift + click to select a range
95f0438
Binary cache: async push_success
autoantwort Feb 15, 2023
9d999d8
Merge branch 'main' into feature/async-binary-cache-push-success
autoantwort Feb 28, 2023
163d9cd
Merge branch 'main' into feature/async-binary-cache-push-success
autoantwort Mar 2, 2023
2a54205
Apply suggestions from code review
autoantwort Mar 2, 2023
0912655
Adapt code review
autoantwort Mar 2, 2023
5d7288c
Update src/vcpkg/binarycaching.cpp
autoantwort Mar 2, 2023
10189ac
Adapt code review
autoantwort Mar 2, 2023
2567607
Remove unnecessary actions_to_push_notifier.notify_all()
autoantwort Mar 2, 2023
ecdd000
Prevent deadlock and don't be on the crtl+c path
autoantwort Mar 2, 2023
8e7ae61
Add and use BGMessageSink to print IBinaryProvider::push_success mess…
autoantwort Mar 3, 2023
850d7c9
Restore old upload message
autoantwort Mar 3, 2023
548be38
Don't join yourself
autoantwort Mar 4, 2023
6dbbf06
Print messages about remaining packages to upload
autoantwort Mar 4, 2023
74b86fd
Localization
autoantwort Mar 5, 2023
5171d3e
Improve messages
autoantwort Mar 5, 2023
d69ed8f
No singleton and explicit calls to wait_for_async_complete()
autoantwort Mar 5, 2023
2df42d5
Merge branch 'main' into feature/async-binary-cache-push-success
autoantwort Mar 8, 2023
5f1786e
Merge branch 'main' into feature/async-binary-cache-push-success
autoantwort Mar 10, 2023
93303c3
Merge branch 'main' into feature/async-binary-cache-push-success
autoantwort Mar 16, 2023
8a26c8b
Merge branch 'main' into feature/async-binary-cache-push-success
autoantwort Mar 19, 2023
aa7e52f
Merge branch 'main' into feature/async-binary-cache-push-success
autoantwort Mar 22, 2023
d46a4d6
Apply code review
autoantwort Mar 22, 2023
5e51718
Trigger Build
autoantwort Mar 22, 2023
a9ac558
No rename dance
autoantwort Mar 22, 2023
4faf674
Print upload to provider only once and not once per provider
autoantwort Mar 22, 2023
b9be8c6
Fix tests
autoantwort Mar 22, 2023
78ca081
Don't create unnecessary strings
autoantwort Mar 31, 2023
579bfa9
Rename to m_published_lock
autoantwort Mar 31, 2023
103968e
BinaryPackageInformation use Optional and make BinaryProviderPushRequ…
autoantwort Mar 31, 2023
dd32416
Merge branch 'main' into feature/async-binary-cache-push-success and …
autoantwort May 31, 2023
b666f94
Add missing files
autoantwort May 31, 2023
15bb503
Add missing includes
autoantwort May 31, 2023
d995bfd
Make BianryCache a unique_ptr
autoantwort May 31, 2023
24cd026
Reduce changes
autoantwort May 31, 2023
92fc76b
Fix output
autoantwort May 31, 2023
3527227
Fix bug
autoantwort May 31, 2023
48305b3
Format
autoantwort May 31, 2023
27fa076
Use lock_guard
autoantwort May 31, 2023
bcd459a
Revert "Use lock_guard"
autoantwort May 31, 2023
f958d36
Use enum
autoantwort May 31, 2023
7a24007
BGMessageSink::print_published apply code review
autoantwort May 31, 2023
50114f9
Merge branch 'main' into feature/async-binary-cache-push-success
autoantwort Jun 14, 2023
ca5f2b1
Merge branch 'main' into feature/async-binary-cache-push-success
autoantwort Aug 19, 2023
eccd9ee
Fix typo
autoantwort Aug 24, 2023
e7837e0
Fix typo in file name
autoantwort Aug 24, 2023
969e7fc
Merge branch 'main' into feature/async-binary-cache-push-success
autoantwort Aug 24, 2023
2d5586f
Merge branch 'main' into feature/async-binary-cache-push-success
autoantwort Oct 11, 2023
809d0b6
Renamings
autoantwort Oct 11, 2023
455e29b
format
autoantwort Oct 12, 2023
03fdfea
Merge branch 'main' into feature/async-binary-cache-push-success
autoantwort Nov 4, 2023
f4bad8c
BinaryCache and std::unique_ptr
autoantwort Nov 4, 2023
26bbbd5
Merge branch 'main' into feature/async-binary-cache-push-success
autoantwort Nov 12, 2023
814e434
BinaryCache: save data in std::unique_ptr so that the object can be m…
autoantwort Nov 14, 2023
290e586
fix
autoantwort Nov 14, 2023
3cc3378
Merge branch 'main' into feature/async-binary-cache-push-success
autoantwort Dec 13, 2023
978ceae
Merge branch 'main' into feature/async-binary-cache-push-success
autoantwort Nov 18, 2024
47b56ce
Merge branch 'main' into feature/async-binary-cache-push-success
autoantwort Dec 25, 2024
061e6e8
Merge branch 'main' into feature/async-binary-cache-push-success
autoantwort Jan 14, 2025
050c51f
Merge remote-tracking branch 'origin/main' into feature/async-binary-…
BillyONeal Jan 15, 2025
8182732
Merge remote-tracking branch 'origin/main' into feature/async-binary-…
BillyONeal Jan 24, 2025
139c7da
Change find_last test to something that find (forward) won't pass.
BillyONeal Dec 9, 2024
4f410f1
Collapse the background work queue system to handle completion of the…
BillyONeal Dec 9, 2024
73b693a
Change BinaryCache and ZipTool's interface to avoid needing to copy o…
BillyONeal Jan 31, 2025
54fe17f
Merge remote-tracking branch 'BillyONeal/contextize-ziptool' into fea…
BillyONeal Jan 31, 2025
27780f9
Fixed upload status being printed to the terminal without synchroniza…
BillyONeal Jan 31, 2025
8e15cf4
Use any_of, put a member FileSystem& back into BinaryCache, make sure…
BillyONeal Jan 31, 2025
21de05a
* Combine submitted/completed counts into one atomic.
BillyONeal Jan 31, 2025
8f372b2
Change submission count message slightly and avoid printing from BG t…
BillyONeal Feb 3, 2025
8ecfdaa
Make the message count pettier.
BillyONeal Feb 3, 2025
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
14 changes: 9 additions & 5 deletions include/vcpkg/base/message-data.inc.h
Original file line number Diff line number Diff line change
Expand Up @@ -2768,11 +2768,15 @@ DECLARE_MESSAGE(SpecifyTargetArch,
"'vcpkg help triplet' is a command line that should not be localized",
"Target triplet. See 'vcpkg help triplet' (default: {env_var})")
DECLARE_MESSAGE(StartCodeUnitInContinue, (), "", "found start code unit in continue position")
DECLARE_MESSAGE(StoredBinariesToDestinations,
(msg::count, msg::elapsed),
"",
"Stored binaries in {count} destinations in {elapsed}.")
DECLARE_MESSAGE(StoreOptionMissingSha, (), "", "--store option is invalid without a sha512")
DECLARE_MESSAGE(SubmittingBinaryCacheBackground,
(msg::spec, msg::count),
"",
"Starting submission of {spec} to {count} binary cache(s) in the background")
DECLARE_MESSAGE(SubmittingBinaryCacheComplete,
(msg::spec, msg::count, msg::elapsed),
"",
"Completed submission of {spec} to {count} binary cache(s) in {elapsed}")
DECLARE_MESSAGE(SuccessfulyExported, (msg::package_name, msg::path), "", "Exported {package_name} to {path}")
DECLARE_MESSAGE(SuggestGitPull, (), "", "The result may be outdated. Run `git pull` to get the latest results.")
DECLARE_MESSAGE(SuggestStartingBashShell,
Expand Down Expand Up @@ -3018,7 +3022,7 @@ DECLARE_MESSAGE(
DECLARE_MESSAGE(UploadingBinariesToVendor,
(msg::spec, msg::vendor, msg::path),
"",
"Uploading binaries for '{spec}' to '{vendor}' source \"{path}\".")
"Uploading binaries for {spec} to {vendor} from {path}")
DECLARE_MESSAGE(UsageTextHere, (), "", "the usage file is here")
DECLARE_MESSAGE(UsageInstallInstructions, (), "", "you can install the usage file with the following CMake")
DECLARE_MESSAGE(UseEnvVar,
Expand Down
38 changes: 36 additions & 2 deletions include/vcpkg/binarycaching.h
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,41 @@ namespace vcpkg
std::unordered_map<std::string, CacheStatus> m_status;
};

struct BinaryCacheSyncState;
struct BinaryCacheSynchronizer
{
using backing_uint_t = std::conditional_t<sizeof(size_t) == 4, uint32_t, uint64_t>;
using counter_uint_t = std::conditional_t<sizeof(size_t) == 4, uint16_t, uint32_t>;
Comment on lines +217 to +218
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why does this depend on size_t?

Copy link
Member

Choose a reason for hiding this comment

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

There are a lot of 64 bit machines without 32 bit atomics, and a lot of 32 bit machines without 64 bit atomics, and I wanted to choose something least likely to put us into lockful atomics world.

static constexpr backing_uint_t SubmissionCompleteBit = static_cast<backing_uint_t>(1)
<< (sizeof(backing_uint_t) * 8 - 1);
static constexpr backing_uint_t UpperShift = sizeof(counter_uint_t) * 8;
static constexpr backing_uint_t SubmittedMask =
static_cast<backing_uint_t>(static_cast<counter_uint_t>(-1) >> 1u);
static constexpr backing_uint_t CompletedMask = SubmittedMask << UpperShift;
static constexpr backing_uint_t OneCompleted = static_cast<backing_uint_t>(1) << UpperShift;

void add_submitted() noexcept;
BinaryCacheSyncState fetch_add_completed() noexcept;
counter_uint_t fetch_incomplete_mark_submission_complete() noexcept;

private:
// This is morally:
// struct State {
// counter_uint_t jobs_submitted;
// bool unused;
// counter_uint_t_minus_one_bit jobs_completed;
// bool submission_complete;
// };
std::atomic<backing_uint_t> m_state = 0;
};

struct BinaryCacheSyncState
{
BinaryCacheSynchronizer::counter_uint_t jobs_submitted;
BinaryCacheSynchronizer::counter_uint_t jobs_completed;
bool submission_complete;
};

// compression and upload of binary cache entries happens on a single 'background' thread, `m_push_thread`
// Thread safety is achieved within the binary cache providers by:
// 1. Only using one thread in the background for this work.
Expand Down Expand Up @@ -249,8 +284,7 @@ namespace vcpkg

BGMessageSink m_bg_msg_sink;
BackgroundWorkQueue<ActionToPush> m_actions_to_push;
std::atomic<int> m_total_packages_to_push = 0;
std::atomic<int> m_total_packages_pushed = 0;
BinaryCacheSynchronizer m_synchronizer;
std::thread m_push_thread;

void push_thread_main();
Expand Down
8 changes: 5 additions & 3 deletions locales/messages.json
Original file line number Diff line number Diff line change
Expand Up @@ -1456,8 +1456,10 @@
"_SpecifyTargetArch.comment": "'vcpkg help triplet' is a command line that should not be localized An example of {env_var} is VCPKG_DEFAULT_TRIPLET.",
"StartCodeUnitInContinue": "found start code unit in continue position",
"StoreOptionMissingSha": "--store option is invalid without a sha512",
"StoredBinariesToDestinations": "Stored binaries in {count} destinations in {elapsed}.",
"_StoredBinariesToDestinations.comment": "An example of {count} is 42. An example of {elapsed} is 3.532 min.",
"SubmittingBinaryCacheBackground": "Starting submission of {spec} to {count} binary cache(s) in the background",
"_SubmittingBinaryCacheBackground.comment": "An example of {spec} is zlib:x64-windows. An example of {count} is 42.",
"SubmittingBinaryCacheComplete": "Completed submission of {spec} to {count} binary cache(s) in {elapsed}",
"_SubmittingBinaryCacheComplete.comment": "An example of {spec} is zlib:x64-windows. An example of {count} is 42. An example of {elapsed} is 3.532 min.",
"SuccessfulyExported": "Exported {package_name} to {path}",
"_SuccessfulyExported.comment": "An example of {package_name} is zlib. An example of {path} is /foo/bar.",
"SuggestGitPull": "The result may be outdated. Run `git pull` to get the latest results.",
Expand Down Expand Up @@ -1597,7 +1599,7 @@
"UpgradeInManifest": "Upgrade upgrades a classic mode installation and thus does not support manifest mode. Consider updating your dependencies by updating your baseline to a current value with vcpkg x-update-baseline and running vcpkg install.",
"_UpgradeInManifest.comment": "'vcpkg x-update-baseline' and 'vcpkg install' are command lines and should not be localized.",
"UpgradeRunWithNoDryRun": "If you are sure you want to rebuild the above packages, run this command with the --no-dry-run option.",
"UploadingBinariesToVendor": "Uploading binaries for '{spec}' to '{vendor}' source \"{path}\".",
"UploadingBinariesToVendor": "Uploading binaries for {spec} to {vendor} from {path}",
"_UploadingBinariesToVendor.comment": "An example of {spec} is zlib:x64-windows. An example of {vendor} is Azure. An example of {path} is /foo/bar.",
"UsageInstallInstructions": "you can install the usage file with the following CMake",
"UsageTextHere": "the usage file is here",
Expand Down
33 changes: 33 additions & 0 deletions src/vcpkg-test/binarycaching.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -469,3 +469,36 @@ Description: a spiffy compression library wrapper
</packages>
)");
}

TEST_CASE ("Synchronizer operations", "[BinaryCache]")
{
{
BinaryCacheSynchronizer sync;
auto result = sync.fetch_add_completed();
REQUIRE(result.jobs_submitted == 0);
REQUIRE(result.jobs_completed == 1);
REQUIRE_FALSE(result.submission_complete);
}

{
BinaryCacheSynchronizer sync;
sync.add_submitted();
sync.add_submitted();
auto result = sync.fetch_add_completed();
REQUIRE(result.jobs_submitted == 2);
REQUIRE(result.jobs_completed == 1);
REQUIRE_FALSE(result.submission_complete);
}

{
BinaryCacheSynchronizer sync;
sync.add_submitted();
REQUIRE(sync.fetch_incomplete_mark_submission_complete() == 1);
sync.add_submitted();
REQUIRE(sync.fetch_incomplete_mark_submission_complete() == 2);
auto result = sync.fetch_add_completed();
REQUIRE(result.jobs_submitted == 2);
REQUIRE(result.jobs_completed == 1);
REQUIRE(result.submission_complete);
}
}
64 changes: 56 additions & 8 deletions src/vcpkg/binarycaching.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2213,6 +2213,44 @@ namespace vcpkg
});
}

void BinaryCacheSynchronizer::add_submitted() noexcept
Copy link
Member

Choose a reason for hiding this comment

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

This starts meaningfully new code.

{
// This can set the unused bit but if that happens we are terminating anyway.
if ((m_state.fetch_add(1, std::memory_order_acq_rel) & SubmittedMask) == SubmittedMask)
{
Checks::unreachable(VCPKG_LINE_INFO, "Maximum job count exceeded");
}
}

BinaryCacheSyncState BinaryCacheSynchronizer::fetch_add_completed() noexcept
{
auto old = m_state.load(std::memory_order_acquire);
backing_uint_t local;
do
{
local = old;
if ((local & CompletedMask) == CompletedMask)
{
Checks::unreachable(VCPKG_LINE_INFO, "Maximum job count exceeded");
}

local += OneCompleted;
} while (m_state.compare_exchange_weak(old, local, std::memory_order_acq_rel));

BinaryCacheSyncState result;
result.jobs_submitted = local & SubmittedMask;
result.jobs_completed = (local & CompletedMask) >> UpperShift;
result.submission_complete = (local & SubmissionCompleteBit) != 0;
return result;
}

BinaryCacheSynchronizer::counter_uint_t BinaryCacheSynchronizer::
fetch_incomplete_mark_submission_complete() noexcept
{
auto state = m_state.fetch_or(SubmissionCompleteBit, std::memory_order_acq_rel);
return (state & SubmittedMask) - ((state & CompletedMask) >> UpperShift);
}

bool BinaryCache::install_providers(const VcpkgCmdArguments& args,
const VcpkgPaths& paths,
MessageSink& status_sink)
Expand Down Expand Up @@ -2468,7 +2506,9 @@ namespace vcpkg
generate_nuspec(request.package_dir, action, m_config.nuget_prefix, m_config.nuget_repo);
}

m_total_packages_to_push++;
m_synchronizer.add_submitted();
msg::println(msg::format(
msgSubmittingBinaryCacheBackground, msg::spec = action.spec, msg::count = m_config.write.size()));
m_actions_to_push.push(ActionToPush{std::move(request), clean_packages});
return;
}
Expand All @@ -2485,10 +2525,12 @@ namespace vcpkg
void BinaryCache::wait_for_async_complete_and_join()
{
m_bg_msg_sink.print_published();
if (m_total_packages_pushed.load() < m_total_packages_to_push.load())
auto incomplete_count = m_synchronizer.fetch_incomplete_mark_submission_complete();
if (incomplete_count != 0)
{
msg::println(msgWaitUntilPackagesUploaded, msg::count = m_total_packages_to_push.load());
msg::println(msgWaitUntilPackagesUploaded, msg::count = incomplete_count);
}

m_bg_msg_sink.publish_directly_to_out_sink();
m_actions_to_push.stop();
if (m_push_thread.joinable())
Expand Down Expand Up @@ -2534,11 +2576,17 @@ namespace vcpkg
m_fs.remove_all(action_to_push.request.package_dir, VCPKG_LINE_INFO);
}

m_bg_msg_sink.println(
msg::format(
msgStoredBinariesToDestinations, msg::count = num_destinations, msg::elapsed = timer.elapsed())
.append_raw(
fmt::format(" ({}/{})", ++(m_total_packages_pushed), m_total_packages_to_push.load())));
auto sync_state = m_synchronizer.fetch_add_completed();
auto message = msg::format(msgSubmittingBinaryCacheComplete,
msg::spec = action_to_push.request.spec,
msg::count = num_destinations,
msg::elapsed = timer.elapsed());
if (sync_state.submission_complete)
{
message.append_raw(fmt::format(" ({}/{})", sync_state.jobs_completed, sync_state.jobs_submitted));
}

m_bg_msg_sink.println(message);
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/vcpkg/dependencies.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -661,7 +661,7 @@ namespace vcpkg
bool RemovePlan::has_non_user_requested() const
{
return Util::any_of(remove,
[](const RemovePlanAction& a) { return a.request_type != RequestType::USER_REQUESTED; });
[](const RemovePlanAction& a) { return a.request_type != RequestType::USER_REQUESTED; });
}

RemovePlan create_remove_plan(const std::vector<PackageSpec>& specs, const StatusParagraphs& status_db)
Expand Down