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.

Sign up for GitHub

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

Jump to bottom

feat(cxx_common): optionally dispose of unselected filesets #5689

Merged
merged 3 commits into from
Jun 7, 2023
Merged

feat(cxx_common): optionally dispose of unselected filesets #5689

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2e2d539
feat(cxx_common): optionally dispose of unselected filesets
shahms Jun 7, 2023
cb61852
Merge branch 'master' into eager-fileset-reclaim
shahms Jun 7, 2023
ab8720a
fix(cxx_common): properly handle out-of-order unselected files
shahms Jun 7, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
93 changes: 58 additions & 35 deletions kythe/cxx/extractor/bazel_artifact_selector.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,10 @@ std::string ToLocalPath(const build_event_stream::File& file) {
}

std::optional<BazelArtifactFile> ToBazelArtifactFile(
const build_event_stream::File& file) {
const build_event_stream::File& file, const RegexSet& allowlist) {
if (!allowlist.Match(file.name())) {
return std::nullopt;
}
std::optional<std::string> uri = ToUri(file);
if (!uri.has_value()) return std::nullopt;
return BazelArtifactFile{
Expand Down Expand Up @@ -643,18 +646,16 @@ absl::optional<BazelArtifact> AspectArtifactSelector::SelectFileSet(
state_.file_sets.Dispose(*file_set_id);
BazelArtifact result = {.label = node.mapped()};
for (const auto& file : fileset.files()) {
if (options_.file_name_allowlist.Match(file.name())) {
if (std::optional<BazelArtifactFile> artifact_file =
ToBazelArtifactFile(file)) {
result.files.push_back(
state_.files.ExtractFile(*std::move(artifact_file)));
}
if (std::optional<BazelArtifactFile> artifact_file =
ToBazelArtifactFile(file, options_.file_name_allowlist)) {
result.files.push_back(
state_.files.ExtractFile(*std::move(artifact_file)));
}
}
for (const auto& child : fileset.file_sets()) {
if (std::optional<FileSetId> child_id =
InternUnlessDisposed(child.id())) {
ExtractFilesInto(*child_id, result.label, result.files);
ExtractFilesInto(*child_id, result.label, &result.files);
}
}
return result;
Expand All @@ -666,31 +667,48 @@ absl::optional<BazelArtifact> AspectArtifactSelector::SelectFileSet(
absl::optional<BazelArtifact> AspectArtifactSelector::SelectTargetCompleted(
const build_event_stream::BuildEventId::TargetCompletedId& id,
const build_event_stream::TargetComplete& payload) {
if (options_.target_aspect_allowlist.Match(id.aspect())) {
BazelArtifact result = {
.label = id.label(),
};
for (const auto& output_group : payload.output_group()) {
// TODO(shahms): optionally prune *all* output groups, matching first.
if (options_.output_group_allowlist.Match(output_group.name())) {
for (const auto& fileset : output_group.file_sets()) {
if (std::optional<FileSetId> file_set_id =
InternUnlessDisposed(fileset.id())) {
ExtractFilesInto(*file_set_id, result.label, result.files);
}
}
}
}
if (!result.files.empty()) {
return result;
BazelArtifact result = {
.label = id.label(),
};
const auto& [selected, unselected] = PartitionFileSets(id, payload);
for (FileSetId file_set_id : selected) {
ExtractFilesInto(file_set_id, result.label, &result.files);
}
if (options_.dispose_unselected_output_groups) {
for (FileSetId file_set_id : unselected) {
ExtractFilesInto(file_set_id, result.label, nullptr);
}
}
if (!result.files.empty()) {
return result;
}
return absl::nullopt;
}

AspectArtifactSelector::PartitionFileSetsResult
AspectArtifactSelector::PartitionFileSets(
const build_event_stream::BuildEventId::TargetCompletedId& id,
const build_event_stream::TargetComplete& payload) {
PartitionFileSetsResult result;
bool id_match = options_.target_aspect_allowlist.Match(id.aspect());
for (const auto& output_group : payload.output_group()) {
auto& output =
(id_match && options_.output_group_allowlist.Match(output_group.name()))
? result.selected
: result.unselected;
for (const auto& fileset : output_group.file_sets()) {
if (std::optional<FileSetId> file_set_id =
InternUnlessDisposed(fileset.id())) {
output.push_back(*file_set_id);
}
}
}
return result;
}

void AspectArtifactSelector::ExtractFilesInto(
FileSetId id, absl::string_view target,
std::vector<BazelArtifactFile>& files) {
std::vector<BazelArtifactFile>* files) {
if (state_.file_sets.Disposed(id)) {
return;
}
Expand All @@ -701,13 +719,20 @@ void AspectArtifactSelector::ExtractFilesInto(
// Record this for future processing.
LOG(INFO) << "NamedSetOfFiles " << state_.file_sets.ToString(id)
<< " requested by " << target << " but not yet disposed.";
state_.pending.emplace(id, target);
if (files != nullptr) {
// Only retain pending file sets if they would've been saved.
state_.pending.emplace(id, target);
} else if (state_.pending.find(id) == state_.pending.end()) {
// But still prefer to retain pending file sets.
state_.file_sets.Dispose(id);
}
return;
}

for (FileId file_id : file_set->files) {
if (std::optional<BazelArtifactFile> file = state_.files.Extract(file_id)) {
files.push_back(*std::move(file));
if (std::optional<BazelArtifactFile> file = state_.files.Extract(file_id);
file.has_value() && files != nullptr) {
files->push_back(*std::move(file));
}
}
for (FileSetId child_id : file_set->file_sets) {
Expand All @@ -719,12 +744,10 @@ void AspectArtifactSelector::InsertFileSet(
FileSetId id, const build_event_stream::NamedSetOfFiles& fileset) {
std::optional<FileSet> file_set;
for (const auto& file : fileset.files()) {
if (options_.file_name_allowlist.Match(file.name())) {
if (std::optional<BazelArtifactFile> artifact_file =
ToBazelArtifactFile(file)) {
FileId file_id = state_.files.Insert(*std::move(artifact_file));
GetOrConstruct(file_set).files.push_back(file_id);
}
if (std::optional<BazelArtifactFile> artifact_file =
ToBazelArtifactFile(file, options_.file_name_allowlist)) {
FileId file_id = state_.files.Insert(*std::move(artifact_file));
GetOrConstruct(file_set).files.push_back(file_id);
}
}
for (const auto& child : fileset.file_sets()) {
Expand Down
16 changes: 15 additions & 1 deletion kythe/cxx/extractor/bazel_artifact_selector.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,10 @@ struct AspectArtifactSelectorOptions {
// Which serialization format version to use.
AspectArtifactSelectorSerializationFormat serialization_format =
AspectArtifactSelectorSerializationFormat::kV2;
// Whether to eagerly drop files and filesets from unselected output groups.
// As this can cause data loss when a file set would have been selected
// by a subsequent target's output group, it defaults to false.
bool dispose_unselected_output_groups = false;
};

/// \brief A BazelArtifactSelector implementation which tracks state from
Expand Down Expand Up @@ -281,8 +285,18 @@ class AspectArtifactSelector final : public BazelArtifactSelector {
const build_event_stream::BuildEventId::TargetCompletedId& id,
const build_event_stream::TargetComplete& payload);

struct PartitionFileSetsResult {
std::vector<FileSetId> selected;
std::vector<FileSetId> unselected;
};
PartitionFileSetsResult PartitionFileSets(
const build_event_stream::BuildEventId::TargetCompletedId& id,
const build_event_stream::TargetComplete& payload);

// Extracts the selected files into the (optional) `files` output.
// If `files` is nullptr, extracted files will be dropped.
void ExtractFilesInto(FileSetId id, absl::string_view target,
std::vector<BazelArtifactFile>& files);
std::vector<BazelArtifactFile>* files);
void InsertFileSet(FileSetId id,
const build_event_stream::NamedSetOfFiles& fileset);

Expand Down