From 53918bed4112993f9a1db27232e2fb8bdd9fbf9b Mon Sep 17 00:00:00 2001 From: Jakob Degen Date: Sun, 18 Aug 2024 15:08:00 -0700 Subject: [PATCH] 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 --- app/buck2_build_api/src/actions/impls/json.rs | 3 ++- app/buck2_build_api/src/artifact_groups.rs | 8 ++++++-- .../rule_defs/artifact/starlark_promise_artifact.rs | 3 ++- .../rule_defs/transitive_set/transitive_set.rs | 5 +++-- .../transitive_set/transitive_set_args_projection.rs | 5 +++-- .../src/artifact_groups/calculation.rs | 5 +++-- 6 files changed, 19 insertions(+), 10 deletions(-) diff --git a/app/buck2_build_api/src/actions/impls/json.rs b/app/buck2_build_api/src/actions/impls/json.rs index 6677d3daa6f6..215e91e2efe8 100644 --- 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 index e79075765a07..dd8e68dedc8a 100644 --- 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), + Promise(Arc), } +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 index 262c319a3ef9..f429826d586a 100644 --- 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 index 001618641062..67a67ff6d39f 100644 --- 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/app/buck2_build_api/src/interpreter/rule_defs/transitive_set/transitive_set_args_projection.rs b/app/buck2_build_api/src/interpreter/rule_defs/transitive_set/transitive_set_args_projection.rs index ba40862e9a75..431e8ab4059a 100644 --- a/app/buck2_build_api/src/interpreter/rule_defs/transitive_set/transitive_set_args_projection.rs +++ b/app/buck2_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 index f99c3ce89de2..2430f4f7e0a1 100644 --- 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()