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.

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

Replace constant hashmaps with const fn #1631

Open
vmx opened this issue Sep 27, 2022 · 1 comment
Open

Replace constant hashmaps with const fn #1631

vmx opened this issue Sep 27, 2022 · 1 comment
Labels
enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed

Comments

@vmx
Copy link
Contributor

vmx commented Sep 27, 2022

Description

Currently there are some constants defined as hashmaps in

pub static ref POREP_MINIMUM_CHALLENGES: RwLock<HashMap<u64, u64>> = RwLock::new(
[
(SECTOR_SIZE_2_KIB, 2),
(SECTOR_SIZE_4_KIB, 2),
(SECTOR_SIZE_16_KIB, 2),
(SECTOR_SIZE_32_KIB, 2),
(SECTOR_SIZE_8_MIB, 2),
(SECTOR_SIZE_16_MIB, 2),
(SECTOR_SIZE_512_MIB, 2),
(SECTOR_SIZE_1_GIB, 2),
(SECTOR_SIZE_32_GIB, 176),
(SECTOR_SIZE_64_GIB, 176),
]
.iter()
.copied()
.collect()
);
pub static ref POREP_PARTITIONS: RwLock<HashMap<u64, u8>> = RwLock::new(
[
(SECTOR_SIZE_2_KIB, 1),
(SECTOR_SIZE_4_KIB, 1),
(SECTOR_SIZE_16_KIB, 1),
(SECTOR_SIZE_32_KIB, 1),
(SECTOR_SIZE_8_MIB, 1),
(SECTOR_SIZE_16_MIB, 1),
(SECTOR_SIZE_512_MIB, 1),
(SECTOR_SIZE_1_GIB, 1),
(SECTOR_SIZE_32_GIB, 10),
(SECTOR_SIZE_64_GIB, 10),
]
.iter()
.copied()
.collect()
);
pub static ref LAYERS: RwLock<HashMap<u64, usize>> = RwLock::new(
[
(SECTOR_SIZE_2_KIB, 2),
(SECTOR_SIZE_4_KIB, 2),
(SECTOR_SIZE_16_KIB, 2),
(SECTOR_SIZE_32_KIB, 2),
(SECTOR_SIZE_8_MIB, 2),
(SECTOR_SIZE_16_MIB, 2),
(SECTOR_SIZE_512_MIB, 2),
(SECTOR_SIZE_1_GIB, 2),
(SECTOR_SIZE_32_GIB, 11),
(SECTOR_SIZE_64_GIB, 11),
]
.iter()
.copied()
.collect()
);
// These numbers must match those used for Window PoSt scheduling in the miner actor.
// Please coordinate changes with actor code.
// https://github.com/filecoin-project/specs-actors/blob/master/actors/abi/sector.go
pub static ref WINDOW_POST_SECTOR_COUNT: RwLock<HashMap<u64, usize>> = RwLock::new(
[
(SECTOR_SIZE_2_KIB, 2),
(SECTOR_SIZE_4_KIB, 2),
(SECTOR_SIZE_16_KIB, 2),
(SECTOR_SIZE_32_KIB, 2),
(SECTOR_SIZE_8_MIB, 2),
(SECTOR_SIZE_16_MIB, 2),
(SECTOR_SIZE_512_MIB, 2),
(SECTOR_SIZE_1_GIB, 2),
(SECTOR_SIZE_32_GIB, 2349), // this gives 125,279,217 constraints, fitting in a single partition
(SECTOR_SIZE_64_GIB, 2300), // this gives 129,887,900 constraints, fitting in a single partition
]
.iter()
.copied()
.collect()
. They are wrapped in a RwLock which is only needed for prodbench, which I even don't know whether anyone is still using/if it still works. So wither prodbench can be removed and if not, I think it doesn't really need the feature of being able to add custom constants.

This means that those hashmaps could be replaces with a constant functions. E.g. POREP_PARTITIONS could be replaces with something like:

pub const fn porep_partitions(sector_size: u64) -> usize {
       match sector_size {
           SECTOR_SIZE_2_KIB => 1,
           SECTOR_SIZE_4_KIB => 1,
            …
           _ => panic!("invalid sector size"),
       }
}

The advantage is that we don't need a lazy_static, but also that if you pass in an unsupported size, it will panic at compile-time, not at runtime.

This change would also remove a lot of boilerplate code. If you search for POREP_PARTITIONS, you'll find many occurrences of:

*POREP_PARTITIONS
    .read()
    .expect("POREP_PARTITIONS poisoned")
    .get(&SECTOR_SIZE_2_KIB)
    .expect("unknown sector size")

which could be replaced by porep_partitions(SECTOR_SIZE_2_KIB).

Acceptance criteria

The hashmaps mentioned above are replaced by const functions.

Risks + pitfalls

Those are part of the public API, so crates like filecoin-proofs-api would also need to be adapted.

Where to begin

Change one of those hashmaps to a const fn and get things compiled.

@vmx vmx added enhancement New feature or request help wanted Extra attention is needed good first issue Good for newcomers labels Sep 27, 2022
@DrPeterVanNostrand
Copy link
Contributor

DrPeterVanNostrand commented Sep 27, 2022

It would be really cool (imho) if we made all proofs' code generic over <const SECTOR_NODES: usize>, which would allow Groth16's usage of the const fn some_setup_param<const SECTOR_NODES: usize>() pattern and would result in many simplifications around proof setup/configuration.

vmx added a commit that referenced this issue Oct 19, 2023
Instead of using one-time initialized HashMaps, use functions, that
directly return the expected values. That reduces the amount of
boilerplate code when those values are used.

Fixes #1631.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants