Variable Storage Improvements#8420
Open
Pesekjak wants to merge 67 commits intoSkriptLang:dev/featurefrom
Open
Conversation
fixed the nodes with no children not resulting in correct format for lists
…dix tree was not thread safe, improved the locking strategy for reading variables in the variables map, replaced tree maps in variables map nodes to hash maps
…ot happen on every clear now
… the variables map node
…ments' into feature/variable-storage-improvements
…ap - live view is now returned
subtree is now cleared only after all write/read operations on it are finished
improved the h2 storage configuration
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The current system for storing variables has significant flaws and runs on really old code, it suffers mainly from poor scalability and also performance, as every variable is deserialized at server startup and stored in a map. This map is then periodically serialized and dumped into a file on the main thread. It also includes some questionable design choices such as limiting a user ability to save large amount of variables in a short period of time or storing every variable on heap (even those that are unused during the whole server lifetime).
Solution
This PR is continuation of #7653 that offers modern alternatives (H2 and SQLite), changes how variables are stored and looked up in memory, responsibilities of variable storages, and makes the variables related code properly thread safe.
VariablesMap is now implemented as a thread safe radix tree where edges are variable name parts, is now exposed as public API and is extensively used in different VariableStorage implementations, more on that later. There is some performance cost for this change, mainly list variables now do not return the underlying map, but a snapshot, and for single variables there is no fast O(1) lookup. Every variable access is now O(N) where N is the depth of the radix tree. I tried implementing such thing with guava's cache but it goes beyond my skill and I am pretty sure the overhead would likely outweighed the performance gains.
VariableStorage now does not only manage the saving of variables externally but also their lifecycle - loading and disposing during server runtime. I introduced new methods
void setVariable(String name, @Nullable Object value)and@Nullable Object getVariable(String name)that allows the variable access within the storage instance.Variables are not stored in a single map, but correct variable storage
VariableStorage findStorage(String name)is chosen depending on the variable name pattern and such variables are stored and managed there. This improves the old design because variable storage can now decide when to load variables and can dispose them to free up space. This is beneficial for users that store large amounts of variables as server start up times and memory usage improve drastically. The design also opens door for more specific variable lifecycle - e.g. storage that shares variables across different servers using a specialized database service, as it all depends on the variable storage implementation, not the static systems in the Variables class.FlatFileStorage works mostly the same as before (some functionality is missing, more info in TODO section).
JdbcStorage allows for any JDBC implementation database, H2, SQLite and MySQL are implemented in this PR.
This is now recommended option over flat file storage as it does not load everything in memory at start up but only when the variables are accessed. Updates to the external database are also efficient as only the changes since the last save are commited.
Classes factory class now provides methods for (de)serialization of multiple items
where if possible the work is happening on the calling thread and only for those serializers that need to be synchronized run on the main thread. This is utilized when saving/loading variables into/from the database.
I hope I did write enough documentation for everything but it could definitely be improved at some places, give feedback if something is unclear (both API and implementation). There shouldn't be many breaking changes on the Java part, as most of the code was either internal or has the same API as before but there are some notable ones: Classes factory class has some signatures of methods changed (just parameter order to match rest of the code), SerializedVariable is now a record, Variables class now doesn't have methods for variable serialization (Classes factory does).
TODO
Optional:
Testing Completed
quick test and small manual tests so far
Supporting Information
I would like someone with better understanding of SQL and JDBC to look at the Jdbc Variable Storage implementation (mainly the queries and database interaction as AI was used).
And also double check the thread safety for VariablesMap but I think it is done correctly.
Completes: none
Related: #1168, #2007, #1478, #3948, #7653 (possibly more, this is from the old PR)
AI assistance: Gemini Pro for improving initial VariablesMap implementation and writing SQL statements