Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Gloot 3.0: Poor performance when serializing #276

Open
snipercup opened this issue Feb 8, 2025 · 8 comments
Open

Gloot 3.0: Poor performance when serializing #276

snipercup opened this issue Feb 8, 2025 · 8 comments
Labels
bug Something isn't working

Comments

@snipercup
Copy link

I am upgrading from Gloot 2 to Gloot 3. I could do most of the work in one day. I'm excited to get Gloot 3 running because it has many benefits compared to 2.0.

The problem I'm running into is the performance when serializing. This is when my serialization happens:

Image

This is what the performance monitor is telling me:

Image

A chunk is about twice the size of what is visible here. You can see a row of furniture that needs to be serialized, and at the top is the kitchen_cupboard.

Image

I don't know why it would call the serialize function 995 times. I don't think I have that many items in a chunk and it will unload at most 3 chunks at a time. This performance issue is not present in Gloot 2.0. If possible, I don't want to serialize the protoset at all since there is only one used in the entire game and I can save it elsewhere if needed.

I am building the protoset here:
https://github.com/snipercup/CataX/blob/96caee3f39b58d7135ceeaacf85dc9f3d7578cdd/Scripts/Runtimedata/RItems.gd#L124-L131

And then I'm calling:
https://github.com/snipercup/CataX/blob/96caee3f39b58d7135ceeaacf85dc9f3d7578cdd/Scripts/item_manager.gd#L691-L692

What can I do to mitigate the performance hit?

@peter-kish
Copy link
Owner

peter-kish commented Feb 9, 2025

Hi!
I didn't spend too much time analyzing your code, but this is the implementation of Inventory.serialize and Inventory._serialize_protoset:

func serialize() -> Dictionary:
    var result: Dictionary = {}

    if protoset == null || _constraint_manager == null:
        return result

    result[_KEY_NODE_NAME] = name as String
    result[_KEY_PROTOSET] = _serialize_protoset(protoset)
    if !_constraint_manager.is_empty():
        result[_KEY_CONSTRAINTS] = _constraint_manager.serialize()
    if !get_items().is_empty():
        result[_KEY_ITEMS] = []
        for item in get_items():
            result[_KEY_ITEMS].append(item.serialize())

    return result


static func _serialize_protoset(protoset: JSON) -> String:
    if !is_instance_valid(protoset):
        return ""
    elif protoset.resource_path.is_empty():
        return protoset.stringify(protoset.data)
    else:
        return protoset.resource_path

Inventory._serialize_protoset is called on only one more place, which is InventoryItem.serialize:

func serialize() -> Dictionary:
    var result: Dictionary = {}

    result[_KEY_PROTOSET] = Inventory._serialize_protoset(protoset)
    if _prototype != null:
        result[_KEY_PROTOTYPE_ID] = str(_prototype.get_prototype_id())
    else:
        result[_KEY_PROTOTYPE_ID] = ""
    if !_properties.is_empty():
        result[_KEY_PROPERTIES] = {}
        for property_name in _properties.keys():
            result[_KEY_PROPERTIES][property_name] = _serialize_property(property_name)

    return result

As you can see, the number of times Inventory._serialize_protoset is called, depends on the number of items inside the inventory and Inventory.serialize is called only once per inventory.

This is also backed by your first screenshot: You have 266 calls to Inventory.serialize and 729 calls to InventoryItem.serialize, which in total make 995 calls to Inventory._serialize_protoset.

The real question here is why is InventoryItem.serialize being called 266 times by your code.

By looking at your stack trace I can see that it is called by get_data once, but get_data is called by get_furniture_data inside a for loop which iterates over collider_to_furniture.values():

func get_furniture_data() -> Array:
	var furniture_data: Array = []
	for furniture in collider_to_furniture.values():
		if is_instance_valid(furniture):
			furniture_data.append(furniture.get_data())
	return furniture_data

I didn't analyze the code any further, but I think it would be worth checking how many values collider_to_furniture.values() returns.

@snipercup
Copy link
Author

That explains the 995 calls. Thank you for looking into it.

I did some more testing, and the 995 calls are probably from a forest. Each tree has an inventory and each inventory has about 3 items. If there are 225 trees in one chunk, then unloading that chunk will cause about 1000 items to be serialized.

Serializing 1000 inventory items should be easy, right? It works perfectly in Gloot 2:

Image

The issue concerning Inventory._serialize_protoset is preventing me from upgrading to Gloot 3. Is there some way to stop it from calling Inventory._serialize_protoset? I think I don't need it since my protoset is build when the game starts anyway.

@peter-kish
Copy link
Owner

peter-kish commented Feb 9, 2025

What exactly do you mean by "the protoset is build when the game starts"? Is it not stored in a JSON resource file? I suspect this might be the key detail here.

In the general case _serialize_protoset shouldn't do anything complex, it only writes the resource path of the protoset in string format. That is, UNLESS the resource path is empty (i.e. not stored in a resource file, as described in the Godot docs for resource_path) and the protoset is generated on the fly:

static func _serialize_protoset(protoset: JSON) -> String:
    if !is_instance_valid(protoset):
        return ""
    elif protoset.resource_path.is_empty():
        return protoset.stringify(protoset.data) # Could be slooow
    else:
        return protoset.resource_path

In that case protoset.data is converted into string and that one gets written instead of the resource path. This conversion could theoretically take some significant time (depending on the protoset size) and it does not happen in v2 because protosets used to be organized differently there.

If you are doing some kind of runtime protoset generation then you could consider saving it as a resource, so that it won't get converted into string every time an inventory or an item gets serialized (not 100% sure how exactly that would work, but I suppose ResourceSaver could be used for that).

@snipercup
Copy link
Author

You're right. the generation and use of the protoset will be the key point here. The protoset is constructed here:
https://github.com/snipercup/CataX/blob/96caee3f39b58d7135ceeaacf85dc9f3d7578cdd/Scripts/Runtimedata/RItems.gd#L124-L131

The resulting save_data dictionary will be the same as what the Gloot readme recommends. I am constructing it at runtime because depending on what mods the user enables, some item definitions may be overwritten. Also, it's true that my item data is stored in json, but I load it into data classes so my game's editor can manipulate it more reliably. This means the json is not edited by hand.

So you're saying that it needs to be saved to a file and then loaded so it has a resource path. This is similar to what I did with item_protoset.tres in Gloot 2 so that should work for me. I will try that.

@snipercup
Copy link
Author

I'm now loading an itemprotosets.json file:


var item_protosets: JSON = preload("res://ItemProtosets.JSON")

I updated my save_protorype function to this:

# Saves the items protoset to ItemManager.item_protosets. We need to do this for the Gloot addon
func save_items_protoset() -> void:
	var save_data: Dictionary = {}

	for item: RItem in itemdict.values():
		save_data[item.id] = item.get_data()  # Store item data using its ID as the key

	ItemManager.update_protoset(save_data)  # Save the dictionary
	
	var save_result = ResourceSaver.save(ItemManager.item_protosets, "res://ItemProtosets.JSON")
	if save_result != OK:
		print_debug("Failed to save updated ItemProtoset resource to:", "res://ItemProtosets.JSON")
	else:
		print_debug("ItemProtoset resource updated and saved successfully to:", "res://ItemProtosets.JSON")

You can see the file here: https://github.com/snipercup/CataX/blob/gloot-upgrade/ItemProtosets.JSON

The issue I had is no longer there. The Inventory._serialize_protoset isn't taking up a lot of time.

However, now I'm having an even bigger problem when de-serializing:

Image

Here, you can see that 6 furniturestaticsrv are deserializing. Some have 2 inventories, so I can see 11 inventories being deserialized. What's strange is that some of the protorype functions are called hundreds of times, even though only 33 InventoryItems are deserializing.

Did I make a mistake in my itemprotosets.json file? Are there too many custom properties?

@peter-kish
Copy link
Owner

I think your protoset file is fine. This time it seems like the problem is in Prototype._find_derived_prototype, which basically does a brute-force search through a tree, which is far from ideal (almost embarrassing).

I'll try to come up with a fix asap.

@peter-kish peter-kish added the bug Something isn't working label Feb 10, 2025
@peter-kish
Copy link
Owner

A small update on this:

After doing some more analysis, it turned out that most of the calls to the Prototype functions is justified, since the protoset you provided contains 286 prototypes and >2000 different properties in total.

That said, I managed to optimize away some of the calls (e.g. Prototype._find_derived_prototype). In my test case where I'm deserializing your protoset I managed to cut the time in half, but your mileage may vary and I'm not sure if this will reach the performance of v2.x (the protosets are a bit more complex in v3.0).

I'll keep looking into this to see if I can come up with some additional optimizations.

@snipercup
Copy link
Author

Thanks for your efforts. I'll keep an eye on your progress. If there's a new version to try then I'll see what the performance is at that time. My game runs fine on Gloot 2.x for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants