Shrink ArtifactGroup

Summary: `ArtifactGroup` currently has size 40 bytes while `Artifact` is 8 bytes. Now `ArtifactGroup` is only 16 bytes. This may have regressed with the recent deferred changes, not sure Reviewed By: stepancheg Differential Revision: D61435902 fbshipit-source-id: e8be08ce8ee21e7670169b0d28ba6cdcf37ca28e
facebook · Aug 18, 2024 · 53918be · 53918be
1 parent 7084dfb
commit 53918be
Show file tree

Hide file tree

Showing 6 changed files with 19 additions and 10 deletions.
diff --git a/app/buck2_build_api/src/actions/impls/json.rs b/app/buck2_build_api/src/actions/impls/json.rs
@@ -9,6 +9,7 @@
 
 use std::io::sink;
 use std::io::Write;
+use std::sync::Arc;
 
 use anyhow::Context;
 use buck2_artifact::artifact::artifact_type::Artifact;
@@ -324,7 +325,7 @@ pub fn visit_json_artifacts(
             }
         }
         JsonUnpack::TransitiveSetJsonProjection(x) => visitor.visit_input(
-            ArtifactGroup::TransitiveSetProjection(x.to_projection_key()?),
+            ArtifactGroup::TransitiveSetProjection(Arc::new(x.to_projection_key()?)),
             None,
         ),
         JsonUnpack::Artifact(_x) => {

diff --git a/app/buck2_build_api/src/artifact_groups.rs b/app/buck2_build_api/src/artifact_groups.rs
@@ -18,6 +18,7 @@ use crate::deferred::calculation::GET_PROMISED_ARTIFACT;
 pub mod registry;
 
 use std::hash::Hash;
+use std::sync::Arc;
 
 use allocative::Allocative;
 pub use artifact_group_values::ArtifactGroupValues;
@@ -26,6 +27,7 @@ use derive_more::Display;
 use dice::DiceComputations;
 use dupe::Dupe;
 use gazebo::variants::UnpackVariants;
+use static_assertions::assert_eq_size;
 
 use self::calculation::EnsureTransitiveSetProjectionKey;
 use crate::artifact_groups::deferred::TransitiveSetKey;
@@ -46,10 +48,12 @@ use crate::artifact_groups::promise::PromiseArtifact;
 )]
 pub enum ArtifactGroup {
     Artifact(Artifact),
-    TransitiveSetProjection(TransitiveSetProjectionKey),
-    Promise(PromiseArtifact),
+    TransitiveSetProjection(Arc<TransitiveSetProjectionKey>),
+    Promise(Arc<PromiseArtifact>),
 }
 
+assert_eq_size!(ArtifactGroup, [usize; 2]);
+
 impl ArtifactGroup {
     /// Gets the resolved artifact group, which is used further downstream to use DICE to get
     /// or compute the actual artifact values. For the `Artifact` variant, we will get the results

diff --git a/app/buck2_build_api/src/interpreter/rule_defs/artifact/starlark_promise_artifact.rs b/app/buck2_build_api/src/interpreter/rule_defs/artifact/starlark_promise_artifact.rs
@@ -10,6 +10,7 @@
 use std::fmt;
 use std::fmt::Debug;
 use std::fmt::Display;
+use std::sync::Arc;
 
 use allocative::Allocative;
 use anyhow::Context as _;
@@ -130,7 +131,7 @@ impl StarlarkPromiseArtifact {
     pub fn as_artifact(&self) -> ArtifactGroup {
         match self.artifact.get() {
             Some(artifact) => ArtifactGroup::Artifact(artifact.dupe()),
-            None => ArtifactGroup::Promise(self.artifact.dupe()),
+            None => ArtifactGroup::Promise(Arc::new(self.artifact.dupe())),
         }
     }
 

diff --git a/app/buck2_build_api/src/interpreter/rule_defs/transitive_set/transitive_set.rs b/app/buck2_build_api/src/interpreter/rule_defs/transitive_set/transitive_set.rs
@@ -9,6 +9,7 @@
 
 use std::fmt;
 use std::iter;
+use std::sync::Arc;
 
 use allocative::Allocative;
 use anyhow::Context;
@@ -241,12 +242,12 @@ impl FrozenTransitiveSet {
         // Reuse the same projection for children sets.
         for v in self.children.iter() {
             let v = TransitiveSet::from_value(v.to_value()).context("Invalid deferred")?;
-            sub_inputs.push(ArtifactGroup::TransitiveSetProjection(
+            sub_inputs.push(ArtifactGroup::TransitiveSetProjection(Arc::new(
                 TransitiveSetProjectionKey {
                     key: v.key().dupe(),
                     projection,
                 },
-            ));
+            )));
         }
         Ok(sub_inputs)
     }

diff --git a/...uck2_build_api/src/interpreter/rule_defs/transitive_set/transitive_set_args_projection.rs b/...uck2_build_api/src/interpreter/rule_defs/transitive_set/transitive_set_args_projection.rs
@@ -10,6 +10,7 @@
 use std::fmt;
 use std::fmt::Display;
 use std::iter;
+use std::sync::Arc;
 
 use allocative::Allocative;
 use anyhow::Context as _;
@@ -241,10 +242,10 @@ impl<'v, V: ValueLike<'v>> CommandLineArgLike for TransitiveSetArgsProjectionGen
             .context("Invalid transitive_set")?;
 
         visitor.visit_input(
-            ArtifactGroup::TransitiveSetProjection(TransitiveSetProjectionKey {
+            ArtifactGroup::TransitiveSetProjection(Arc::new(TransitiveSetProjectionKey {
                 key: set.key().dupe(),
                 projection: self.projection,
-            }),
+            })),
             None,
         );
 

diff --git a/app/buck2_build_api_tests/src/artifact_groups/calculation.rs b/app/buck2_build_api_tests/src/artifact_groups/calculation.rs
@@ -8,6 +8,7 @@
  */
 
 use std::collections::HashMap;
+use std::sync::Arc;
 
 use buck2_analysis::analysis::calculation::AnalysisKey;
 use buck2_artifact::artifact::artifact_type::Artifact;
@@ -199,12 +200,12 @@ async fn test_ensure_artifact_group() -> anyhow::Result<()> {
     let mut dice = dice.commit().await;
 
     let result = dice
-        .ensure_artifact_group(&ArtifactGroup::TransitiveSetProjection(
+        .ensure_artifact_group(&ArtifactGroup::TransitiveSetProjection(Arc::new(
             TransitiveSetProjectionKey {
                 key: set.key.dupe(),
                 projection: 0,
             },
-        ))
+        )))
         .await?
         .iter()
         .cloned()