mononoke: UnodeManifest does not need path in walker key

Summary: The path isn't needed to load the UnodeManifest, it's only needed as route information for compression etc

Reviewed By: aslpavel

Differential Revision: D25187547

fbshipit-source-id: b8199a81c5dae4caceed5d455fa6a9bbc3f037ac
This commit is contained in:
Alex Hornby 2020-11-30 03:21:15 -08:00 committed by Facebook GitHub Bot
parent b7a7c27a94
commit b046b8940b
4 changed files with 44 additions and 47 deletions

View File

@ -353,7 +353,7 @@ create_graph!(
),
(
UnodeManifest,
PathKey<UnodeKey<ManifestUnodeId>>,
UnodeKey<ManifestUnodeId>,
[UnodeFileChild(UnodeFile), UnodeManifestChild(UnodeManifest), UnodeManifestParent(UnodeManifest), LinkedChangeset(Changeset)]
),
(UnodeMapping, ChangesetId, [RootUnodeManifest(UnodeManifest)]),
@ -592,7 +592,7 @@ impl Node {
Node::SkeletonManifest(k) => k.blobstore_key(),
Node::SkeletonManifestMapping(k) => k.blobstore_key(),
Node::UnodeFile(k) => k.blobstore_key(),
Node::UnodeManifest(PathKey { id, path: _ }) => id.blobstore_key(),
Node::UnodeManifest(k) => k.blobstore_key(),
Node::UnodeMapping(k) => k.blobstore_key(),
}
}
@ -627,7 +627,7 @@ impl Node {
Node::SkeletonManifest(_) => None,
Node::SkeletonManifestMapping(_) => None,
Node::UnodeFile(_) => None,
Node::UnodeManifest(PathKey { id: _, path }) => Some(&path),
Node::UnodeManifest(_) => None,
Node::UnodeMapping(_) => None,
}
}
@ -663,7 +663,7 @@ impl Node {
Node::SkeletonManifest(k) => Some(k.sampling_fingerprint()),
Node::SkeletonManifestMapping(k) => Some(k.sampling_fingerprint()),
Node::UnodeFile(k) => Some(k.sampling_fingerprint()),
Node::UnodeManifest(PathKey { id, path: _ }) => Some(id.sampling_fingerprint()),
Node::UnodeManifest(k) => Some(k.sampling_fingerprint()),
Node::UnodeMapping(k) => Some(k.sampling_fingerprint()),
}
}
@ -679,7 +679,7 @@ mod tests {
#[test]
fn test_node_size() {
// Node size is important as we have lots of them, add a test to check for accidental changes
assert_eq!(64, size_of::<Node>());
assert_eq!(48, size_of::<Node>());
}
#[test]

View File

@ -74,15 +74,13 @@ impl FromStr for UnodeFlags {
}
}
impl FromStr for PathKey<UnodeKey<ManifestUnodeId>> {
impl FromStr for UnodeKey<ManifestUnodeId> {
type Err = Error;
fn from_str(s: &str) -> Result<Self, Self::Err> {
let parts: Vec<_> = s.split(NODE_SEP).collect();
let inner = ManifestUnodeId::from_str(parts[0])?;
let flags = UnodeFlags::from_str(parts[1])?;
let path = check_and_build_path(NodeType::UnodeManifest, &parts[1..])?;
let id = UnodeKey { inner, flags };
Ok(Self { id, path })
Ok(UnodeKey { inner, flags })
}
}
@ -329,8 +327,8 @@ mod tests {
assert_eq!(
node_type,
&parse_node(&format!(
"UnodeManifest{}{}{}{:b}{}{}",
NODE_SEP, SAMPLE_BLAKE2, NODE_SEP, 0b00000011, NODE_SEP, SAMPLE_PATH
"UnodeManifest{}{}{}{:b}",
NODE_SEP, SAMPLE_BLAKE2, NODE_SEP, 0b00000011
))?
.get_type()
);

View File

@ -179,10 +179,7 @@ pub struct WalkState {
visited_skeleton_manifest: StateMap<SkeletonManifestId>,
visited_skeleton_manifest_mapping: StateMap<InternedId<ChangesetId>>,
visited_unode_file: StateMap<UnodeInterned<FileUnodeId>>,
visited_unode_manifest: StateMap<(
InternedId<Option<MPathHash>>,
UnodeInterned<ManifestUnodeId>,
)>,
visited_unode_manifest: StateMap<UnodeInterned<ManifestUnodeId>>,
visited_unode_mapping: StateMap<InternedId<ChangesetId>>,
// Count
visit_count: [AtomicUsize; NodeType::COUNT],
@ -470,15 +467,12 @@ impl VisitOne for WalkState {
flags: k.flags,
},
),
Node::UnodeManifest(k) => self.record_with_path(
Node::UnodeManifest(k) => self.record(
&self.visited_unode_manifest,
(
&k.path,
&UnodeInterned {
id: self.unode_manifest_ids.interned(&k.id.inner),
flags: k.id.flags,
},
),
&UnodeInterned {
id: self.unode_manifest_ids.interned(&k.inner),
flags: k.flags,
},
),
Node::UnodeMapping(bcs_id) => {
if let Some(id) = self.bcs_ids.get(bcs_id) {

View File

@ -845,19 +845,21 @@ async fn bonsai_to_unode_mapping_step<V: VisitOne>(
if let Some(root_unode_id) = root_unode_id {
let mut edges = vec![];
checker.add_edge(&mut edges, EdgeType::UnodeMappingToRootUnodeManifest, || {
Node::UnodeManifest(PathKey::new(
UnodeKey {
inner: *root_unode_id.manifest_unode_id(),
let manifest_id = *root_unode_id.manifest_unode_id();
checker.add_edge_with_path(
&mut edges,
EdgeType::UnodeMappingToRootUnodeManifest,
|| {
Node::UnodeManifest(UnodeKey {
inner: manifest_id,
flags,
},
WrappedPath::Root,
))
});
})
},
|| Some(WrappedPath::Root),
);
Ok(StepOutput(
checker.step_data(NodeType::UnodeMapping, || {
NodeData::UnodeMapping(Some(*root_unode_id.manifest_unode_id()))
NodeData::UnodeMapping(Some(manifest_id))
}),
edges,
))
@ -922,8 +924,8 @@ async fn unode_manifest_step<V: VisitOne>(
ctx: &CoreContext,
repo: &BlobRepo,
checker: &Checker<V>,
path: WrappedPath,
key: UnodeKey<ManifestUnodeId>,
key: &UnodeKey<ManifestUnodeId>,
path: Option<&WrappedPath>,
) -> Result<StepOutput, Error> {
let unode_manifest = key.inner.load(ctx, repo.blobstore()).await?;
let linked_cs_id = *unode_manifest.linknode();
@ -943,10 +945,11 @@ async fn unode_manifest_step<V: VisitOne>(
}
for p in unode_manifest.parents() {
checker.add_edge(
checker.add_edge_with_path(
&mut edges,
EdgeType::UnodeManifestToUnodeManifestParent,
|| Node::UnodeManifest(PathKey::new(UnodeKey { inner: *p, flags }, path.clone())),
|| Node::UnodeManifest(UnodeKey { inner: *p, flags }),
|| path.cloned(),
);
}
@ -954,12 +957,15 @@ async fn unode_manifest_step<V: VisitOne>(
for (child, subentry) in unode_manifest.subentries() {
match subentry {
UnodeEntry::Directory(id) => {
let mpath_opt =
WrappedPath::from(MPath::join_element_opt(path.as_ref(), Some(child)));
checker.add_edge(
checker.add_edge_with_path(
&mut edges,
EdgeType::UnodeManifestToUnodeManifestChild,
|| Node::UnodeManifest(PathKey::new(UnodeKey { inner: *id, flags }, mpath_opt)),
|| Node::UnodeManifest(UnodeKey { inner: *id, flags }),
|| {
path.map(|p| {
WrappedPath::from(MPath::join_element_opt(p.as_ref(), Some(child)))
})
},
);
}
UnodeEntry::File(id) => {
@ -968,10 +974,9 @@ async fn unode_manifest_step<V: VisitOne>(
EdgeType::UnodeManifestToUnodeFileChild,
|| Node::UnodeFile(UnodeKey { inner: *id, flags }),
|| {
Some(WrappedPath::from(MPath::join_element_opt(
path.as_ref(),
Some(child),
)))
path.map(|p| {
WrappedPath::from(MPath::join_element_opt(p.as_ref(), Some(child)))
})
},
);
}
@ -1470,8 +1475,8 @@ where
Node::UnodeFile(id) => {
unode_file_step(&ctx, &repo, &checker, &id, walk_item.path.as_ref()).await
}
Node::UnodeManifest(PathKey { id, path }) => {
unode_manifest_step(&ctx, &repo, &checker, path, id).await
Node::UnodeManifest(id) => {
unode_manifest_step(&ctx, &repo, &checker, &id, walk_item.path.as_ref()).await
}
Node::UnodeMapping(bcs_id) => {
bonsai_to_unode_mapping_step(&ctx, &repo, &checker, bcs_id, enable_derive).await