Optimize process_ledger()

[rob-solana]

No description provided.

[rob-solana]

cargo test process_ledger_around_window -- --nocapture --test solana::bank

[rob-solana]

Toly, wanna try this on your 96-core monster? I think this is 25% faster than the WINDOW_SIZE chunking version.

[garious]

How long does this take to execute? I run the few test suite constantly when developing via cargo watch 'test --libs' and want that instantaneous feedback. If this takes more than a second to execute, I'd prefer you move in to a #[bench].

[rob-solana]

I don't want to land the benchmark, it takes awhile

[garious]

Overall developer productively is a higher priority than fullnode startup time.

[garious]

Is this comment relevant anymore? In no, can you delete?

[rob-solana]

I think the comment is still relevant, because I don't know the answer

[garious]

The answer is that we don't need to register any entry IDs for security purposes. We register them for only these reasons:

• So that the server doesn't need to compare against every signature there's ever been to ensure a transaction is not a duplicate. It can look up a set of signatures by last_id.
• So that a client knows its transaction is only valid on a ledger containing its transaction's last_id or later.
• So that a client knows that if a transaction wasn't accepted after a certain amount of time that it'll never be accepted.

[garious]

Any way that can be implemented with higher-level Vec functions. Maybe something like:

let mut (left, right) = v.split_at(tail_idx);
right.extend(left);
right

Alternatively, if you move this code into a separate function and write a test that demonstrates your intent here, I don't need to grok the implementation - just the test.

[rob-solana]

what you've written is equivalent, with more mallocs()

[garious]

Right, it's more complex than the simplest correct version, which is why you'd create a separate function and test for it

[garious]

which would leave someone else the opportunity to either further optimize it and see that it still meets your requirements, as well as leave space for someone to add a benchmark and possibly determine the compiler already implements your optimization.

[rob-solana]

 fn test_rotate_vector() {
        let test = vec![4, 1, 2, 3];
        let etest = vec![4, 1, 2, 3];

        let (left, right) = etest.split_at(1);
        let mut right = right.to_vec();
        right.extend(left);

        assert_eq!(rotate_vector(test, 1), right);

        let now = Instant::now();
        for _ in 0..100_000 {
            let etest = vec![4, 1, 2, 3];
            let (left, right) = etest.split_at(1);
            let mut right = right.to_vec();
            right.extend(left);
        }
        eprintln!("split_at {}", duration_as_us(&now.elapsed()));

        let now = Instant::now();
        for _ in 0..100_000 {
            let test = vec![4, 1, 2, 3];
            rotate_vector(test, 1);
        }
        eprintln!("rotate {}", duration_as_us(&now.elapsed()));
    }

split_at 97145
rotate 70582

[garious]

One less malloc: right.chain(left).to_vec()

[rob-solana]

will have a go

[rob-solana]

ug, fighting with types in rust to get chain() to work on right...

[rob-solana]

878 |         let right = right.chain(left).to_vec();
    |                           ^^^^^
    |
    = note: the method `chain` exists but the following trait bounds were not satisfied:
            `&mut &[{integer}] : std::iter::Iterator`
            `&mut [{integer}] : std::iter::Iterator`

[garious]

Untested, but the chain docs suggest you need an iter():
right.iter().chain(left).to_vec()