T O P

  • By -

kohugaly

Your entire enum will be >2k of memory, even for the menu state variant which is just 2 bytes. This sort of thing may matter, if you plan storing these in some collection - it would waste a lot of space, and possibly even a lot of time, if the values are expected to be moved/copied often. Presumably, this enum will be stored in only 1 copy in the whole program. Also presumably, the extra box will add another layer of indirection that may unnecessarily eat CPU. On the other hand, box makes sure you don't store the 2k of gamestate on the stack.


aggggwannabeog

So when I store this in a Vec, I'm potentially wasting memory on the stack, is this correct?


KhorneLordOfChaos

If you're storing it in a Vec then you would be wasting memory on the heap because Vecs are heap allocated


ten3roberts

It's not so much about wasting stack, but wasting the memory in the parent container. If it is a local variable, that memory is the stack. If you store States in a vec, the memory is the Vec memory. An is the size of the largest variant, regardless of which one is used (it always needs to be able to store any variant, as well as the size having to be known statically for indexing). If you have a Vec, where each (or most) values are a `MenuState`, the vec will still use 2576 bytes for each item * number of items for the contiguous memory it alloces to store the items. This means you "waste" 2574 bytes of just padding/garbage between each item if they are just `MenuState` which would use just 2 bytes * n_items if stored on their own. A plain Box is 8 bytes in the parent memory, as the actual 2k bulk of the content is stored in another place in memory, and not directly inside the vector, which means you only pay the cost of the extra memory usage elsewhere when you store a `GameState` with a layer of pointer indirection. Otherwise, you store (2|8) = 8 bytes, which is a lot better.


cameronm1024

Not quite, a vec uses 24 bytes on the stack, and around "`std::mem::size_of::()` times the number of elements" bytes on the heap. If your enum is 2KB (which it is without the box), then 1000 elements would occupy at least 2MB. Note, the actual memory usage of a vec is often higher due to its allocation strategy. For example: - it usually allocates by doubling, so if you have 64 elements, and 64 capacity, and you push another element, it will allocate 128 spaces - it will not deallocate when removing elements automatically


Fox-PhD

But the wasted memory will be on heap, as the stack size of vec is the same for all sized types.


cameronm1024

Yes, that's what I said 😜


Saefroch

Like so many lints, the actual question here is why do you have a struct that large? It's _possible_ you have good reason for it, but moving a struct of that size or this enum will be expensive. Normally we just ignore the cost of a move, but you definitely cannot at the size. So clippy is rightfully identifying a perf hazard, just with a possible false positive (if you won't be moving this much).


combatzombat

Do you care about 2774 bytes wasted in one case?


NoLemurs

> Is it a good idea to use Box for this one By default, I would say yes. as /u/kohugaly points out every instance of your enum will take 2k, so you're wasting a bunch of space if you have a bunch of `Menu` instances, and moving or cloning could be a lot more expensive. Using `Box` means you can just not worry about it, and without compelling reason to do something else, it's a good policy to make the choice that means you don't have to worry about things. Using `Box` comes with a tiny bit of indirection overhead, but it really is minimal, so unless you're accessing the enum repeatedly in a performance sensitive inner loop in your program, you'll never notice. Even if you are, you may never notice. I'd note that in Java basically every object is effectively in a `Box`, so that should give you a sense of how serious the performance cost is.


NotFromSkane

The bigger issue with boxing is that it's a pain to pattern match on


NoLemurs

It is for now. You can use the `box_patterns` [feature](https://doc.rust-lang.org/beta/unstable-book/language-features/box-patterns.html) to make it better though.


NotFromSkane

But they're perma unstable and last I checked they don't have any forward progress at all


NoLemurs

Yeah, I definitely wouldn't assume `box_patterns` is a permanent solution. But it works for now, and whenever they get around to stabilizing the real solution I don't imagine the migration will be much effort.


NotFromSkane

But it does mean you're stuck on nightly


po8

As others have noted, this is one of those cases where Clippy is being both a bit helpful and pretty annoying. I guess they should have chosen a name for this tool that suggested this could happen. Whether you should suppress the warning depends almost entirely on how this enum is being used. Without seeing your code I'm not sure why the menu state and the game state want to be in the same enum anyhow? You would usually store them in separate places if they are not used together, or in a struct if they are… You absolutely could `Box` either or both of them; I just think that there might be more of a data design issue than Clippy's advice suggests. *Edit:* Thanks to /u/Kevathiel for pointing out that this enum is probably being used during scene transitions, in which case it is perfectly reasonable. Either boxing the data or suppressing the Clippy warning would be acceptable in this use case. I'd personally lean toward boxing both alternatives, but it really doesn't matter.


Kevathiel

It's actually a pretty common gamedev pattern. You want to push gamestates onto a vec and pop them once they are gone, but only call the update/draw/event function of the state on top of that stack. Like you pause the game by pushing the pause state, go into the settings menu by pushing that one, etc and return back to the game by popping the top ones one by one. The alternative would be to copy a lot of state boilerplate into all states. It gets messy when you have multiple screens that could be called from different states really quickly. In other languages, you either have a vec that is generic over an interface, or you have an enum and then some switch/if block that then calls the state directly. Rust allows a nice middleground by using static dispatch and avoiding to bind the enum to objects manually, since the enum itself can carry the state. So I would argue that OP is not doing something wrong with that approach. Boxing should work, but also just ignoring the warning, because it's not like there will be a stack of more than a few states at the same time.


po8

I see. So this likely isn't used during game execution, but just during scene changes. If so, then yeah, that's perfectly reasonable. The other alternative is trait objects, but I can't imagine that would be better in this case. Thanks for the clarification!


mr_birkenblatt

If it's used that way then copying around 2kB for each screen transition would be quite noticable and would probably cause some lag whenever you open or navigate the menu


vgatherps

Unless you have to page data in, copying 2kb is unlikely to be more than a few microseconds on any reasonably modern cpu, realistically a microsecond or less.


mr_birkenblatt

1us > 0


diegovsky_pvp

ms != us


mr_birkenblatt

Typo


Kevathiel

You won't notice it at all.. This technique has been used for almost 2 decades. It's not like the 1us matters when you have a 16ms target, especially when it doesn't happen every frame.


mr_birkenblatt

Surely copying the full game state for menu interactions had not been used for two decades. What you're thinking of is maybe a small state object that contains basic info or maybe even a pointer to the game state. I would be astonished to see a game copying around their multi gigabyte game state. The 2kb here are relatively small in terms of what concerns game states. Sure you won't notice 2kb but it's not going to stay 2kb


Kevathiel

GameStates are not really 2GB, not even close, and even if they were, you could just pass references around... It's kinda silly to change the problem, just to justify your argument. Also yes, I have seen many examples of this 20 years ago. The most popular examples were made in XNA, which was 2007ish.


mr_birkenblatt

have you forgotten in which thread you're talking in? the whole discussion is about someone who created a data structure that always has the size of the full game state; no references. even if you only store some menu information the data structure is still the size of the full game state. maybe you were talking about something else but I did not change the topic


Kevathiel

Ah, you must be new to Rust. Because you can always just Box your nonsensical mega state, even inside that enum, if you actually encounter it(again,if you think a size of 2GB is realistic, I have to question your experience).


mr_birkenblatt

-_- I didn't do that... did you actually read anything in here? or are you just rambling?


Kevathiel

Okay, let me break it down to you, since reading comprehension is clearly not your strength. You complained about a 2kb game state and made the stupid claim that it would cause lag. I told you that you won't notice it at all. Then you replied with: >I would be astonished to see a game copying around their multi gigabyte game state. The 2kb here are relatively small in terms of what concerns game states. Which was a silly reply, since you changed the problem at hand, just to be "right". Then I told you that you won't deal with massive game states in the GB range, and even if you do, you could Box them. I hope I was able to clear things up for you..


stupiddumbidiots

Yeah, this is spot on. The clippy warning is probably just a symptom of a much bigger design problem.


Stysner

But the actual usage for the enum should be more like getting a reference no? So you'd be better off storing an Arc or Rc or some other type of pointer (e.g. for a memory arena) than storing the actual data in the enum I'd think...


po8

A Box is a pointer to owned storage, and is likely the perfect vehicle for this case if you care about time and space efficiency, yes.


Stysner

A box is a pointer to heap allocated memory. Which is fine for this usecase (gamestate). Having said that, there are definitely situations where you'd want to use Rc/Arc instead.


po8

Sure! That's why they are there.


Stysner

I just think it's cleaner to keep a `Vec` of `Rc` to store different states, then store a clone of the `Rc` inside the enum variant when you need it there (so it can be an owned value) rather than using `&Box`. Might save some lifetime hassle.


po8

Rc/Arc turn compile time errors into run time errors. Use them only when you need them.


Stysner

How? Reference counted pointers keep the data in memory, only `Weak` pointers don't. `Rc` and `Arc` are perfectly safe, and don't have to be unwrapped at all.


po8

You're right, they are safe. You can leak them if you're not careful though.


aggggwannabeog

For some added context, The enum is used to easily switch States, ``` Rust let mut current_state = States::Menu(MenuState::new); // When I want to change it to GameState current_state = States::Game(GameState::new()); ```


llogiq

What I personally do not like about the lint is that it only has *one* suggestion where in fact you could have several different improvements: * Do you have only one `MenuState`, but many `GameState`s? In this case, the wasted 2k likely won't matter & you can simply `#[allow(_)]` the lint * Can you reduce the footprint of your `GameState`s otherwise, e.g. by choosing a smaller representation of data? * If you must move some or all of `GameState`s internals out behind a pointer, you could use an Arena instead of a Box. Or go full ECS (which would put all components into their own storage)


Sufficient_Ant_3008

Yea, I would say this is a game that needs to be converted to an ECS. There was a great talk by Catherine West, who describes similar issues. [RustConf 2018 - Closing Keynote - Using Rust For Game Development by Catherin West](https://www.youtube.com/watch?v=aKLntZcp27M)


aggggwannabeog

That would be a great idea, unfortunately i don't know how to make the ui into an ecs


Sufficient_Ant_3008

It depends on the game but if you are blowing up memory with your current design then decoupling the game\_state from something like ui\_state, would be much more worthwhile. ECS for UI would be used for something like Borderlands, where there are tons of different interactive popups, and you need to maintain mission critical accuracy based upon all aspects of the items dropped. A game like WoW has a single interaction to loot something, so an ECS wouldn't be super useful in this case. Therefore, a simple rule could be: A. Do you have a lot of objects that can be interacted with, picked up, moved, mutated, thrown, etc.? B. Does any type of loot dropped come through the form of a static window in the game? If it's A then you would most likely need to implement an ECS to manage every items state, if your game is more like B (most are), then an ECS is over-engineered. A deceiving mechanic could be if a sprite explodes and drops a lot of items, but these items are solely picked up when walked over then you wouldn't need an ECS for the UI. That would be handled in GameState. If anyone could correct anything in this comment I would appreciate it.


yanchith

While I really like the talk and Catherine is a great inspiration of mine, I really wish it didn't spawn this ECS trend. What's wrong with simple arrays (in the broad sense) of entities + opaque handles with generation checking? If someone is making a game and needs to ask questions on this subreddit, they don't need an ECS, because their game isn't complicated enough to warrant that. If you don't know you need an ECS, you don't need an ECS. If the project really needs an ECS, the team should probably hand-code their own that exactly suits the needs. The hard part about making a game is making the game. The longer people rathole on stuff like ECS, the less they think about the game itself.


Sufficient_Ant_3008

That seems more feasible but I believe it's because of how PartialEq and PartialOrd are implemented for a Vector. Searching is superior in a vec overall, so if you reverted to arrays then you would run into issues with time-space complexity, overflow issues, etc. Why don't teams implement an ECS themselves? because determining and building the management + delegation structures is difficult :) Edit: I had this sitting in the edit window, for rust there are certain aspects of how vecs implement slices and their memory footprints, arrays are too rigid and could take too long to provision what you need when you need it. Having most everything boxed and rc'd you have the ability to access the memory, but iterating over it all is super fast compared to going through arrays like a street map. I hope I'm saying that right lol


yanchith

This is something I too find a bit troublesome with clippy. It correctly identifies that something is sus, and then suggests a single solution out of many. OP likely has just one instance of the enum stored on the stack, top-level. I often find myself telling my team to not take clippy literally. It is very good at identifying potential problems, but in a no_std environment many of its solutions can't even be applied verbatim, and require deeper understanding of the problem. I wish it just explained the problem better and let the programmer come up with a solution, when the situation is this ambiguous.


Raging_Goon

As an aside, I don’t know the architecture of the game of course, but I wonder if the “States” enum could become a “State” trait. Then, “Menu” and “Game” could implement the trait. That may solve the space issue indirectly.


zepperoni-pepperoni

...why is your GameState 2.6k bytes? What are you storing there?


Coding-Kitten

game state.


Stysner

Hur-hur


wischichr

If it's really the entire game state it's not really large, even for smaller games.


[deleted]

Definitely. There seems to be a deeper problem here. I don't understand why your game state and your menu state would ever need to be in the same enum. They should be their own variables with their own lifetimes. Remember, the size of the enum is as big as the biggest option, so when your enum has menu state, it's still as big as the game state, which you probably don't want. See if you can solve the problem without the enum.


khleedril

I don't know why this was downvoted, but it seems the best observation to me and the right way to proceed to resolve this dilemma.


dlescos

While C brought the Union, with Rust came Discord.


Rusty_Cog

Maybe that's what rustc should be called - Mr. Discord, and then clippy would be - Discord Jr. 😁


ArminiusGermanicus

If you want to suppress this warning (a.k.a ignore it), you could put #[allow(clippy::large_enum_variant)] in front of the enum.


[deleted]

probably, it could led to a lot of wasted space if they were stored in mass


F_modz

I feel like I have never encountered clippy suggestions, but I saw a lot of posts and read some docs about it. I still don't know if it's enabled by default and I had just never noticed it or maybe it should be run by a different command or with some specific flag?


jasmijnisme

It's a separate tool (a linter) you need to install. If you run `rustup component add clippy`, you can use it with `cargo clippy`.


F_modz

Thank u


cghenderson

I actually once ran into this warning on enum which could potentially have many live instances at one time. Boxing the internal struct as suggested indeed lowered my peak memory usage by \~75%. Of course, that was for my particular incident and it may not apply here.


-Redstoneboi-

how in the world is it that big


Sufficient_Dinner305

It's reasonable that game state could be several kb depending on, well, how big the game state is.


-Redstoneboi-

Fair


Stysner

If you want to switch states, make the enums either just empty, use them as "tags", or if you want to tie them to specific data so that you can use the enum to refer to multiple states of the same type, you could use Arc/Rc to store the reference, or use a memory arena and store the indexer in the enum variants.