Previously, the Load Balancer was constructed even when disabled, incurring unnecessary overhead.

Did you actually benchmark it? I suspect your reading of a config variable is actually orders of magnitude slower than the trivial LoadBalancer initialisation.

For the second point, the queue is the Rust code's responsibility, and I don't think we should be exposing internal access to it. You can either do some operation that implicitly clears the queues (like col.db.execute("update col set mod=mod")), or define a public method in the Rust layer to toggle the setting on and off.

I suspect your reading of a config variable is actually orders of magnitude slower than the trivial LoadBalancer initialisation.

If you're referring to the self.get_config_bool(BoolKey::LoadBalancerEnabled) checks preceding load_balancer.add_card/remove_card calls, this can indeed be addressed by removing the configuration checks entirely. The existing if let Some(load_balancer) = &mut card_queues.load_balancer check is sufficient, as the configuration variable and card_queues.load_balancer state are synchronized after the self._backend.clear_study_queues() call.

For the second point, the queue is the Rust code's responsibility, and I don't think we should be exposing internal access to it.

I have mixed feelings about using db magic like col.db.execute("update col set mod=mod") in the _set_enable_load_balancer function. If you don't find using an RPC to change the state of card_queues appealing, can you elaborate on the "public method in the Rust layer"? The thing is that we need to rebuild the card_queues after changing the mw.col.load_balancer_enabled (or just clear it so it will be rebuild when needed).

^ Removed the self.get_config_bool(BoolKey::LoadBalancerEnabled) checks before add_card/remove_card and changed the &/&mut to .as_ref()/.as_mut()

Now we are cooking! After 785f88a this PR improves performance in comparison with main:

• Execution speed is faster because LoadBalancerEnabled is no longer checked on every get_scheduling_states call (Benefits all users regardless of load balancer configuration)
• Reduced memory usage for users with disabled LB - load_balancer allocation is eliminated

The final step is to agree on how to synchronize the config variable and the LoadBalancer. I believe the ClearStudyQueues RPC is the most reliable/effective way to synchronize these. That said, I acknowledge this is a rather direct/"straight" approach.

Win-win solution, in my opinion, will be to make the LoadBalancerEnabled a real GUI option (enabled by default) in the Preferences: Review window. Operations with options in the Preferences have the Op::UpdatePreferences type that forces to clear the self.state.card_queues in the maybe_clear_study_queues_after_op method on preference change.

I'm afraid I think exposing a ClearStudyQueues RPC is the wrong approach here. I still think this too obscure an option to expose in the GUI, but I won't object to you adding it to GetPreferences/SetPreferences if you wish to do that instead. The GUI would ignore the extra field, but it could be updated in code. If you don't want to go that route, a routine that sets the option on or off is going to be cleanest.

If you don't want to go that route, a routine that sets the option on or off is going to be cleanest.

I'm not quite sure I understand your suggestion. Could you clarify what you mean by a "routine that sets the option on or off"? As mentioned earlier, the only way to reconstruct the load_balancer object is to clear the entire card_queues object (forcing reconstruction when needed).

The Rust code should be resetting the queues as the config option is changed. You can do this either by adding it to the preferences set/get code, or by creating a backend method set_load_balancer_enabled() that sets the config var and clears the queues at the same time. The caller in Python shouldn't need to know about the queues.

I am unsure about the return types for set_load_balancer_enabled and SetLoadBalancerEnabled. Additionally, in self.set_config_bool(BoolKey::LoadBalancerEnabled, input.val, false), should the undoable parameter be set to false or true?

You can make it undoable provided the queue resetting is done by adding that new operation to requires_study_queue_rebuild(), so queues are also reset when it's undone. This would be done instead of explicitly clearing them in the routine you've added. The return type should be the same as set_config_bool().

I considered using crate::prelude::*;, but doing so would require removing both use crate::collection::Collection; and use crate::error;, as well as replacing all instances of error::Result<*> with Result<*>.

Thanks for your patience Yuki - looks good. For future reference, such import changes are fine, but please do them in a separate commit (in the same PR) to make things easier to review.

Thank you for your thoughtful review and feedback on this PR! Your insights were extremely helpful.

@YukiNagat0

You added the following in ftl/core/actions.ftl (which is then used in rslib/src/ops.rs):

actions-toggle-load-balancer = Toggle Load Balancer

But is this string exposed somewhere in the ui? I wanted to edit the german translation in pontoon but wanted to see where the string is being used first.

Edit: I built the latest code from main today.

But is this string exposed somewhere in the ui?

It appears when you undo the command:

1. Open the console and disable LB: mw.col.load_balancer_enabled = False
2. Close the console
3. Press Ctrl+Z for undo action
4. In the bottom-left corner the message should appear: Toggle Load Balancer undone

Interesting. After following your first 2 steps, no undo action is registered in the edit menu. Ctrl+z doesn't do anything either.

I tested it in a new profile as well, where pp(mw.col.load_balancer_enabled) returned True at the beginning. After setting it to False with the command you provided, it returns False. No undo action though.

Is this a bug?

idk,

Seems like it appears only after you rebuild the queues (e.g. clicking on any deck from main screen)

I see. Clicking on a deck afterwards does work.

Edit: I opened an issue in #3918.

The undo menus won't be updated automatically after running a command directly in the debug console. I overlooked the translation when merging in the PR - had I noticed, I would have suggested removing it, since it's a little silly to ask translators to translate something most people won't even see.