Serialization

[mokun]

Currently we are using Java serialization for saving and loading the simulation session.

If we switch to JSON serialization, it will give us developers more debugging insights.

See this for one of the earliest discussion on using JSON for serialization.

@bevans2000, do you favor using Jackson for this ?

[bevans2000]

Jackson is definitely the defacto library for JSON generation. Obviously the main issue I see is the Mars sim is underpinned by an object graph with many references. I don't know how they would be represented in JSON; there would need to be some form of global unique identifier.

[bevans2000]

However Jackson can require the addition of Annotations to the classes plus I'm not sure how good it is at referencing.
GSON by Google is an alternative, https://github.com/google/gson. This apparently needs no change to the code.
There is also json- io which claims to be better than GSON, https://github.com/jdereg/json-io/blob/master/user-guide.md.
IMHO the benefits of a JSON output have to be carefully considered to the effort. Create a branch and give one of them a go, GSON seems to be least intrusive to the existing code.

[mokun]

GSON by Google is an alternative

We could first transition to GSON.

But @Urwumpe would say that GSON use reflection and is slow.

Q: If Java reflection is going to be phased out in near future since it's based on sun.reflect, should we still consider that ?

[Urwumpe]

The decision should be based on the amount of data you need to store and how fast. Maybe reflection would be OK, since we only use it for loading and saving things, but I would suggest using something that operates on compile time by generating the necessary code from annotations. I don't recommend using low-level APIs or custom code there, because this optimization kills agility.

Reflection will still be supported in Java - only the Sun API will drop away, java.lang.reflect will stay the API that you should be using.

More important than that work should be caring for the responsibilities of the objects and getting the vertical structure of the simulation fixed - currently many classes access other classes far outside their packages, increasing cohersion. When this is fixed, loading and saving should be easier as well, since things no longer need to be associated in a complex way, but instead just follow the layers for communication. The tasks are the worst offenders there.

[bevans2000]

I agree, there is a lot of spaghetti coding with classes knowing the internals on others. Probably should try and enforce some package only scoped classes to enforce separation.Also I have noticed logic in base classes specialised for the subclasses. This is never good for maintenance.

[mokun]

@bevans2000 ,

Yes. Sorry about the spaghetti codes as I was focused only on adding features without considering the best architecture that should have been deployed :D

Do by all means refactor how it should be done in such a way that bring stability to the simulation.

[bevans2000]

What about approaching this as a Technical Debt project? Start capturing areas of improvements to reduce cohesion and increase separation as Improvement Issues as they are discovered. Each one could outline a proposed solution open for discussion.
Then we can pick them off when time/opportunities permit.
Maybe even create a new Issue Label to distinguish these Issues fro genuine functionality improvements.

[bevans2000]

I think the best approach is to create a new Project within the repo and create cards with potential changes/improvements/reworks.
Creating Issues might become too numerous. Having Project cards will be more lightweight. We can sort the cards accordingly to priority.
This will allow a gradual progression of changes, evolution not revolution.

[mokun]

Project cards will be more lightweight.

We tried setting up projects in the past. It's just the mechanism doesn't seem to be as intuitive as we want and not having a nice UI with color template like other project management tools out there.

But do by all means utilizing it to see how we may make the best use of that.

[bevans2000]

Good point. I had forgotten that GitHub projects are really just tracking Issues.

[mokun]

If we adopt Java 16, we could quickly switch to using Record for storing days for Java serialization.

Minimal changes are needed.

See https://inside.java/2021/03/12/simpler-serilization-with-records/

[bevans2000]

Java 16 is just a stepping stone release before the LTS release of Java 17. Yes Records are good addition and will simplify a lot of code but they are immutable so there would only be limited use in Mars simi , e.g. config classes, basic POJOs

[Urwumpe]

I think records would also be better suited for new projects, not legacy.

Also, the big issue for serialization here isn't the framework for serialization, but actually the data that we want to store. I can annote everything for jackson-databind, thats not the deal. The problem is still, to get the dependencies between the classes structured into a tree of compositions and define every other association between the classes as plain Object Identity References.

[mokun]

@Urwumpe @bevans2000

Btw, Java Record’s components can be serialized. See https://inside.java/2021/10/21/sip24/

The problem is still, to get the dependencies between the classes structured into a tree of compositions and define every other association between the classes as plain Object Identity References.

Yes this is true. We need to prune the object graph/tree and loose up tightly coupled relationship.

Specifically, I found PhysicalCondition having to keep ways too many data members and we can reorganize and decompose further what's in this class.

[mokun]

How do you like adopting the use of microstream in mars-sim, a well developed, reliable java persistence framework out there ?

See [intro](See https://www.infoq.com/news/2021/09/microstream-5-is-open-source/
).

They just made it open source and besides Java, they have an android version as well.

I was told that it's quite lightweight. So we can use it to replace our heavy duty Inventory class.

In future, we may use it for saving the physical health and the accomplishment of a person, as well building/vehicle maintenance history.

There's also a module for writing the serialized data into a JSON format if needed.

[Urwumpe]

I think its a bad idea to include a new framework now. It will break a lot, for minimal gains. Maybe it makes sense to test in a branch, how many changes are coming there, but I would recommend to first write a lot of unit tests for the Inventory class to ensure that the new framework underneath does not break the top-level stuff. Such tests are never a bad idea anyway and if the new framework turns out to improve things a lot, we can migrate easier to it without having to fear many bugs.

[mokun]

Yes tests are good and immensely helpful in the long run as the project gets complex. It's just a little mundane to write them.

btw, a month ago or so, we've figured how to adopt the use of junit 5 in conjunction with running the old and existing junit 4 test cases.

So at least it's a progress :D.

One problem with the Inventory class is that currently nearly half of all CPU utilization in mars-sim seems to be spent on calling the methods inside this class.

It's highly cpu intensively. See this.

So Inventory is too unoptimized for its two primary tasks of simply STORING and RETRIEVING resources.

May be we can figure out ways to optimize it by cutting down many of those unnecessary instanceof calls to see what happen next.

[mokun]

I would recommend to first write a lot of unit tests for the Inventory class t

btw, thanks for seeing both sides in weighting the options before us.

Guess we can rewrite some of the unit tests for the Inventory class

But it's still quite difficult to understand this class on how all those caches work (and whether the cache is dirty or not), as @bevans2000 has allured to it this way at #348 (comment)

So I'd like to test microstream to see if it's a good fit for us. See manual at https://docs.microstream.one/manual/storage/index.html

Since it works in Android, I would imagine it's lightweight enough for our use case.

[mokun]

See also this stackoverflow post regarding microstream.

So we can use microstream for dual purposes:

1. As a replacement for the Inventory class that handle the retrieval and storage of resources - the methods in Inventory get called so much that they use almost 50% of cpu resources.
2. As a replacement for Java serialization/deserialization of various objects. On top of that , we currently use gzip to compress the serialized objects. We would like to use microstream to convert them into plain JSON files.

[bevans2000]

Microstreams are interesting but they are still a binary storage format like Serialization. If we are to move away from Serialization (I would like to) it needs to provide some benefit. The benefit I feel would be a non-binary format to allow tweaking. JSON would give us that BUT JSON does not support object graphs; and we have a complex object graph.
However, I don't see how this can help with Inventory. The high CPU in Inventory is just a result of it growing and now has become very complex and integral to lots of the logic. I think that Inventory is too big and over the top. Some irregularities in Inventory I thought of whilst working on the loading logic:

1. Why does it hold Units? e.g. to find parked vehicles at a settlement we consult the Inventory. It would be quicker to have a parked vehicle list.
2. Only Equipment should be stored in Inventory BUT referenced/retrieved via the EquipmentType, not an Integer and certainly not a ```Class``. Adding new Equipment requires coding so using an enum is fine.
3. Consolidate the 2 ways to reference resources, AmountResource and Integers into a single set of methods. Typesafe key class like AmountResource` would be clearer.
4. Do the same to Parts and hold them explicitly to remove the common pattern of looking if the resource identifier is less than or more than a RESORUCE_IDENTIFIER to use different methods.

We probably need a separate Discussion on the future the the Inventory class. This undoubtedly would need a gradual transition and not a big bang; probably a set of Issues with a Project to cover them.

[mokun]

The benefit I feel would be a non-binary format to allow tweaking.

microstream does allow exporting to and importing from various formats. See https://docs.microstream.one/manual/storage/import-export.html

It also supports REST interface. See https://docs.microstream.one/manual/storage/rest-interface/index.html

[mokun]

a separate Discussion on the future the the Inventory class.

Sure. Just opened a new issue focusing on tackling the complexity of the Inventory class:

• Divide and conquer the Inventory class #463

[bevans2000]

@mokun I saw last night a few imports for microstream creep into the code base. I think this work should be done on a dedicated branch to avoid breaking the master branch as I don't think it will be a quick piece of work.

[mokun]

done on a dedicated branch to avoid breaking the master branch

Sure. Just started a brand new microstream branch at https://github.com/mars-sim/mars-sim/tree/microstream

[bevans2000]

Super. It will be good to keep this separate as I expect there will be some tears along the way.

[mokun]

Another library that we may use is Chronicle-Wire.

Some said it can be a drop-in replacement of Java serialization.

Don't know how easy it is for us to use it.

See

1. https://foojay.io/today/high-performance-java-serialisation/
2. https://github.com/OpenHFT/Chronicle-Wire

It's capable of using various formats such as binary, yaml, json, etc for serialization