Skip to content

Variable Storage Improvements#8420

Open
Pesekjak wants to merge 67 commits intoSkriptLang:dev/featurefrom
Pesekjak:feature/variable-storage-improvements
Open

Variable Storage Improvements#8420
Pesekjak wants to merge 67 commits intoSkriptLang:dev/featurefrom
Pesekjak:feature/variable-storage-improvements

Conversation

@Pesekjak
Copy link
Contributor

@Pesekjak Pesekjak commented Jan 31, 2026

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

@Nullable Map<String, Object> deserialize(Set<SerializedVariable> variables)
@Nullable Set<SerializedVariable> serialize(Map<String, @Nullable Object> variables)

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

  • JUnit tests for the other classes
  • save task delay and period should be configurable
  • required changes per save should be configurable per variable storage
  • conversions from v2_0_beta3 and v2_1 for FlatFileStorage (it is from 2017, maybe we can finally disable it, thoughts?)
  • better logging when deserialization fails for some variables
  • migrate variables to according storage if it does not accept the name of the variable
  • database table name can not be configured for jdbc storages
  • information in config.sk is outdated
  • backups!
  • tests for SQLite
  • tests for H2
  • write up an API tutorial for registering custom storage types
  • document all the possible options for databases or have annotations for a documentation system

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

@Pesekjak Pesekjak requested review from a team as code owners January 31, 2026 22:50
@Pesekjak Pesekjak added feature Pull request adding a new feature. variables Related to variables and/or storing them. labels Jan 31, 2026
@Pesekjak Pesekjak requested review from APickledWalrus and cheeezburga and removed request for a team January 31, 2026 22:50
@skriptlang-automation skriptlang-automation bot added the needs reviews A PR that needs additional reviews label Jan 31, 2026
@Pesekjak Pesekjak added breaking changes Pull or feature requests that contain breaking changes (API, syntax, etc.) don't merge me !! For pull requests that should not be merged due to some outstanding dispute, conflict or dependency. labels Jan 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking changes Pull or feature requests that contain breaking changes (API, syntax, etc.) don't merge me !! For pull requests that should not be merged due to some outstanding dispute, conflict or dependency. feature Pull request adding a new feature. needs reviews A PR that needs additional reviews variables Related to variables and/or storing them.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants