config system prototype #9318
config system prototype #9318
Conversation
pascalkuthe
added
the
S-experimental
pascalkuthe
force-pushed
the
config_refactor
branch
4 times, most recently
from
8e88b06
to
a1f2a3a
Compare
pascalkuthe
force-pushed
the
config_refactor
branch
from
a1f2a3a
to
87266b0
Compare
| /// this is a reimplementation of dynamic dispatch that only stores the | ||
| /// information we need and stores everythin inline. Values that are smaller or | ||
| /// the same size as a slice (2 usize) are also stored inline. This avoids | ||
| /// significant overallocation when setting lots of simple config | ||
| /// options (integers, strings, lists, enums) |
There was a problem hiding this comment.
Is avoiding an allocation for every config item the only reason we're doing this? Has there been a known performance bottleneck on config memory access?
Trying to understand where this is coming from. This is much harder to understand than just boxing everything, and I was not under the impression that config was in a hot path.
There was a problem hiding this comment.
I actually do kind of think its pretty hot. We read config fields on every single frame for a large amount of config options (at least a hundred) sometimes multiple times. It also reduces memory consumption quite a bit which is more relevant if each config option is set as default, by the user per languages and potentially on the document (so in total up to 4 times). Really this sort of thing ought to be built into rust. There are some efforts around that.
You don't really need to be aware of how this work tough just treat like Box<dyn Any> when reading the rest of the code its really just optimizing access
There was a problem hiding this comment.
Ah that's true, I didn't consider they are read in the rendering hot loop, good point 👍
not anywhere near ready to merge. The general system is mostly complete.
What is missing is porting the entire editor to use the new config system and to add an adapter for the legacy serde config.
I already ported the language server configuration in 27b8257 which should help showoff how this works out in practice.
Rough todo list:
helix_core::configto dedicatedhelix_configcrate (feels more appropriate as the codebase has grown a lotserde_json::Valuetohelx_config::Valueconversion (going though serde is likely unessesairily complicated implementing from directly should work)