Skip to content

Commit

Permalink
Performance optimization, using fixed-length vector for migration fea…
Browse files Browse the repository at this point in the history
…ture IDs
  • Loading branch information
tao-stones committed Dec 1, 2024
1 parent d5623ee commit 9192989
Show file tree
Hide file tree
Showing 3 changed files with 122 additions and 46 deletions.
55 changes: 53 additions & 2 deletions builtins-default-costs/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,16 @@ lazy_static! {
};
}

lazy_static! {
/// A static list of builtin programs' migration feature IDs.
pub static ref MIGRATION_FEATURES_ID: Vec<Pubkey> = {
BUILTIN_INSTRUCTION_COSTS
.values()
.filter_map(|builtin_cost| builtin_cost.core_bpf_migration_feature)
.collect()
};
}

pub fn get_builtin_instruction_cost<'a>(
program_id: &'a Pubkey,
feature_set: &'a FeatureSet,
Expand All @@ -162,10 +172,22 @@ pub fn get_builtin_instruction_cost<'a>(
.map(|builtin_cost| builtin_cost.native_cost)
}

pub fn get_builtin_core_bpf_migration_feature(program_id: &Pubkey) -> Option<Option<Pubkey>> {
/// Given a program pubkey, returns:
/// - None, if it is not in BUILTIN_INSTRUCTION_COSTS dictionary;
/// - Some<None>, is builtin, but no associated migration feature ID;
/// - Some<usize>, is builtin, and its associated migration feature ID
/// index in MIGRATION_FEATURES_ID.
pub fn get_builtin_migration_feature_index(program_id: &Pubkey) -> Option<Option<usize>> {
BUILTIN_INSTRUCTION_COSTS
.get(program_id)
.map(|builtin_cost| builtin_cost.core_bpf_migration_feature)
.map(|builtin_cost| {
builtin_cost.core_bpf_migration_feature.map(|id| {
MIGRATION_FEATURES_ID
.iter()
.position(|&x| x == id)
.expect("must be known migration feature ID")
})
})
}

#[cfg(test)]
Expand Down Expand Up @@ -202,4 +224,33 @@ mod test {
.is_none()
);
}

#[test]
fn test_get_builtin_migration_feature_index() {
assert!(get_builtin_migration_feature_index(&Pubkey::new_unique()).is_none());

assert_eq!(
get_builtin_migration_feature_index(&compute_budget::id()),
Some(None)
);

let feature_index = get_builtin_migration_feature_index(&solana_stake_program::id());
assert_eq!(
MIGRATION_FEATURES_ID[feature_index.unwrap().unwrap()],
feature_set::migrate_stake_program_to_core_bpf::id()
);

let feature_index = get_builtin_migration_feature_index(&solana_config_program::id());
assert_eq!(
MIGRATION_FEATURES_ID[feature_index.unwrap().unwrap()],
feature_set::migrate_config_program_to_core_bpf::id()
);

let feature_index =
get_builtin_migration_feature_index(&address_lookup_table::program::id());
assert_eq!(
MIGRATION_FEATURES_ID[feature_index.unwrap().unwrap()],
feature_set::migrate_address_lookup_table_program_to_core_bpf::id()
);
}
}
89 changes: 53 additions & 36 deletions runtime-transaction/src/compute_budget_instruction_details.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use {
crate::compute_budget_program_id_filter::{ComputeBudgetProgramIdFilter, ProgramKind},
solana_builtins_default_costs::MIGRATION_FEATURES_ID,
solana_compute_budget::compute_budget_limits::*,
solana_sdk::{
borsh1::try_from_slice_unchecked,
Expand All @@ -16,7 +17,7 @@ use {

#[cfg_attr(test, derive(Eq, PartialEq))]
#[cfg_attr(feature = "dev-context-only-utils", derive(Clone))]
#[derive(Default, Debug)]
#[derive(Debug)]
pub(crate) struct ComputeBudgetInstructionDetails {
// compute-budget instruction details:
// the first field in tuple is instruction index, second field is the unsanitized value set by user
Expand All @@ -28,16 +29,32 @@ pub(crate) struct ComputeBudgetInstructionDetails {
num_compute_budget_instructions: u32,
num_builtin_instructions: u32,
num_non_builtin_instructions: u32,
// A list of migration feature IDs for builtin instructions
maybe_builtin: Vec<Pubkey>,
// The vector of counters, matching the size of the static vector MIGRATION_FEATURE_IDS,
// tracks the usage frequency of each feature ID, with each counter representing the
// number of times its corresponding feature ID is referenced in this transaction.
migrating_builtin: Vec<u32>,
}

impl Default for ComputeBudgetInstructionDetails {
fn default() -> Self {
Self {
requested_compute_unit_limit: None,
requested_compute_unit_price: None,
requested_heap_size: None,
requested_loaded_accounts_data_size_limit: None,
num_compute_budget_instructions: 0,
num_builtin_instructions: 0,
num_non_builtin_instructions: 0,
migrating_builtin: vec![0; MIGRATION_FEATURES_ID.len()],
}
}
}

impl ComputeBudgetInstructionDetails {
pub fn try_from<'a>(
instructions: impl Iterator<Item = (&'a Pubkey, SVMInstruction<'a>)>,
) -> Result<Self> {
let mut filter = ComputeBudgetProgramIdFilter::new();

let mut compute_budget_instruction_details = ComputeBudgetInstructionDetails::default();
for (i, (program_id, instruction)) in instructions.enumerate() {
match filter.get_program_kind(instruction.program_id_index as usize, program_id) {
Expand All @@ -61,12 +78,18 @@ impl ComputeBudgetInstructionDetails {
1
);
}
ProgramKind::MaybeBuiltin {
core_bpf_migration_feature,
ProgramKind::MigratingBuiltin {
core_bpf_migration_feature_index,
} => {
compute_budget_instruction_details
.maybe_builtin
.push(core_bpf_migration_feature);
saturating_add_assign!(
*compute_budget_instruction_details
.migrating_builtin
.get_mut(core_bpf_migration_feature_index)
.expect(
"migrating feature index within range of MIGRATION_FEATURE_IDS"
),
1
);
}
}
}
Expand Down Expand Up @@ -170,48 +193,37 @@ impl ComputeBudgetInstructionDetails {
}

#[inline]
fn num_builtin_instructions(&self, additional_num_builtin_instructions: usize) -> u32 {
self.num_builtin_instructions.saturating_add(
additional_num_builtin_instructions
.try_into()
.expect("Count too large for u32"),
)
fn num_builtin_instructions(&self, additional_num_builtin_instructions: u32) -> u32 {
self.num_builtin_instructions
.saturating_add(additional_num_builtin_instructions)
}

#[inline]
fn num_non_builtin_instructions(&self, additional_num_non_builtin_instructions: usize) -> u32 {
self.num_non_builtin_instructions.saturating_add(
additional_num_non_builtin_instructions
.try_into()
.expect("Count too large for u32"),
)
fn num_non_builtin_instructions(&self, additional_num_non_builtin_instructions: u32) -> u32 {
self.num_non_builtin_instructions
.saturating_add(additional_num_non_builtin_instructions)
}

#[inline]
fn num_non_compute_budget_instructions(&self) -> u32 {
self.num_builtin_instructions
.saturating_add(self.num_non_builtin_instructions)
.saturating_add(
self.maybe_builtin
.len()
.try_into()
.expect("size of maybe_builtin too large for u32"),
)
.saturating_add(self.migrating_builtin.iter().sum())
.saturating_sub(self.num_compute_budget_instructions)
}

#[inline]
fn calculate_default_compute_unit_limit(&self, feature_set: &FeatureSet) -> u32 {
if feature_set.is_active(&feature_set::reserve_minimal_cus_for_builtin_instructions::id()) {
// evaluate MaybeBuiltin
// evaluate MigratingBuiltin
let (additional_num_non_builtin_instructions, additional_num_builtin_instructions) =
self.maybe_builtin.iter().fold(
self.migrating_builtin.iter().enumerate().fold(
(0, 0),
|(migrated, builtin), core_bpf_migration_feature| {
if feature_set.is_active(core_bpf_migration_feature) {
(migrated + 1, builtin)
|(migrated, builtin), (index, count)| {
if feature_set.is_active(&MIGRATION_FEATURES_ID[index]) {
(migrated + count, builtin)
} else {
(migrated, builtin + 1)
(migrated, builtin + count)
}
},
);
Expand Down Expand Up @@ -597,11 +609,16 @@ mod test {
&Pubkey::new_unique(),
),
]);
let expected_details = Ok(ComputeBudgetInstructionDetails {
let feature_id_index = MIGRATION_FEATURES_ID
.iter()
.position(|id| *id == feature_set::migrate_stake_program_to_core_bpf::id())
.unwrap();
let mut expected_details = ComputeBudgetInstructionDetails {
num_non_builtin_instructions: 1,
maybe_builtin: vec![feature_set::migrate_stake_program_to_core_bpf::id()],
..ComputeBudgetInstructionDetails::default()
});
};
expected_details.migrating_builtin[feature_id_index] = 1;
let expected_details = Ok(expected_details);
let details =
ComputeBudgetInstructionDetails::try_from(SVMMessage::program_instructions_iter(&tx));
assert_eq!(details, expected_details);
Expand Down
24 changes: 16 additions & 8 deletions runtime-transaction/src/compute_budget_program_id_filter.rs
Original file line number Diff line number Diff line change
@@ -1,18 +1,22 @@
// static account keys has max
use {
agave_transaction_view::static_account_keys_frame::MAX_STATIC_ACCOUNTS_PER_PACKET as FILTER_SIZE,
solana_builtins_default_costs::{get_builtin_core_bpf_migration_feature, MAYBE_BUILTIN_KEY},
solana_builtins_default_costs::{get_builtin_migration_feature_index, MAYBE_BUILTIN_KEY},
solana_sdk::pubkey::Pubkey,
};

#[derive(Clone, Copy, Debug, PartialEq)]
pub(crate) enum ProgramKind {
NotBuiltin,
Builtin { is_compute_budget: bool },
Builtin {
is_compute_budget: bool,
},
// Builtin program maybe in process of being migrated to core bpf,
// if core_bpf_migration_feature is activated, then the migration has
// completed and it should not longer be considered as builtin
MaybeBuiltin { core_bpf_migration_feature: Pubkey },
MigratingBuiltin {
core_bpf_migration_feature_index: usize,
},
}

pub(crate) struct ComputeBudgetProgramIdFilter {
Expand Down Expand Up @@ -44,11 +48,11 @@ impl ComputeBudgetProgramIdFilter {
return ProgramKind::NotBuiltin;
}

get_builtin_core_bpf_migration_feature(program_id).map_or(
get_builtin_migration_feature_index(program_id).map_or(
ProgramKind::NotBuiltin,
|core_bpf_migration_feature| match core_bpf_migration_feature {
Some(core_bpf_migration_feature) => ProgramKind::MaybeBuiltin {
core_bpf_migration_feature,
Some(core_bpf_migration_feature_index) => ProgramKind::MigratingBuiltin {
core_bpf_migration_feature_index,
},
None => ProgramKind::Builtin {
is_compute_budget: solana_sdk::compute_budget::check_id(program_id),
Expand Down Expand Up @@ -125,8 +129,12 @@ mod test {
index += 1;
assert_eq!(
test_store.get_program_kind(index, &migrating_builtin_pubkey),
ProgramKind::MaybeBuiltin {
core_bpf_migration_feature: migration_feature_id
ProgramKind::MigratingBuiltin {
core_bpf_migration_feature_index:
solana_builtins_default_costs::MIGRATION_FEATURES_ID
.iter()
.position(|&x| x == migration_feature_id)
.unwrap(),
}
);
}
Expand Down

0 comments on commit 9192989

Please sign in to comment.