StructureSpawn causing "circular structure" error on JSON.stringify()

SemperRabbit

btw, while we're still discussing the whole objects in Memory thing, I submitted a PR for the error described in my original post... https://github.com/screeps/engine/pull/92

artch

@gankdalf This cost is negligible, JSON.parse will be called only on global resets.

artch

@semperrabbit I don't think that saving game objects to memory will be considered bad behavior after this change. We may even remove this warning from the docs then. It's perfectly safe to store objects in memory if they are parsed correctly. Yes, it makes memory serialization process a bit more abstract for players who are new to JavaScript, but Screeps is not about teaching people how to JavaScript, it's a MMO RTS game.

SemperRabbit

@artch ok, so I finally got around to testing adding a function as a 2nd param to JSON.parse for creating Memory... I used the same test code above, except for editing runTest(). below are my results, and the edited portion of the test code. Although the CPU usage is not as awful as the stringify, its still pretty bad...

number of tests: 50
avg without parser: 12.5632
avg with parser: 22.9115
avg difference: 10.3483

global.parser = function parser(key, val) {
	return ((_.isObject(val) && val.id) ? Game.getObjectById(val.id) : val);
};
global.runTest = function runTest(){
	if(!global.statTest)global.statTest=[];
	var stat = [];

	/* no replacer test */
	var start=Game.cpu.getUsed();
	JSON.parse(RawMemory.get());
	stat.push(Game.cpu.getUsed()-start);

	/* replacer test */
	start=Game.cpu.getUsed();
	JSON.parse(RawMemory.get(), parser);
	stat.push(Game.cpu.getUsed()-start);
	statTest.push(stat);
};

artch

@semperrabbit It doesn't matter how bad it is, if it's being run only once a day (with IVM).

SemperRabbit

I understand that this would be less of an impact with IVM, but we still need to keep in mind the multiple global resets due to development and multiple code pushes in a short period of time. 10 CPU'ers would also be heavily impacted by this addition, as they may only have 0.5 CPU spare per tick to recover their bucket.

artch

@semperrabbit It highly depends on how you use your Memory. In our synthetic tests the increase is only about 15%. And if you don't save objects to Memory (i.e. if there is no __objId key), it's even less.

ags131

I don't like the idea of having to pay extra CPU, even if its a small amount, for a feature I won't need or use, an option to disable the 'feature' would be nice. The cost would be much larger if you store many keys in Memory vs if you had few keys. I would likely end up implementing my own parsing just to avoid it.

Gankdalf

As it is, I am already having to spend 7-10 CPU from simply loading my code when a global resets. Add in my setup processes and cache creations and I am already using 12-16 CPU when my global resets. This is with under 1KB of memory and 30-40KB of code in IVM.

I have been a little concerned with this lately as a 10 CPU user (I don't want to pay for a subscription again till I actually need it), since it seems to be caused by simply loading my code. What happens when I have a 100KB codebase with enough in memory to use 22 CPU on this new parse? Will I be losing 50-60 CPU each time I make a change to my code?

Assuming I am using near my CPU limit each tick, this would run me out of bucket in as little as 250 commits. Memory parsing is not "free" in IVM, it is just not much of an issue outside of development.

Why add a function to my codebase that I absolutely don't need and don't want? Why not just educate the newbies better instead of allowing them to be even more confused by what serialization is?

If they get used to serializing game objects like this, how much more confused will they be when they try to serialize one of their own objects and the instance is destroyed?

SemperRabbit

Another major issue with this is in allowing a full game object be serialized successfully into Memory, It will add excessive bloat to a player's Memory, especially in the beginning phase of learning how to play the game when they're most vulnerable to CPU waste. Imagine a newbie seeing this is a thing, and saving all of the creeps in their room in Memory. that's easily 500 characters per creep, and think of all of the nested objects caused by it that would exacerbate the CPU penalty to using the new parser...

artch

@ags131 We can add RawMemory.autoCreateObjects boolean property which is true by default, but you can change it to false before accessing Memory first time on the tick.

artch

@gankdalf

If they get used to serializing game objects like this, how much more confused will they be when they try to serialize one of their own objects and the instance is destroyed?

They will be confused anyway, since Memory object is going to become persistent, i.e. you can store live data in it without any serialization, and it will be parsed from JSON only on global restarts.

artch

@semperrabbit We could store such objects in the short {__objId: 'abc123'} form, but it would disable debugging using Memory inspector UI then.

Gankdalf

@artch If you added a log in their console for serializing such objects instead of helping them do it, I think it would go a long ways.

SemperRabbit

It would also prevent debugging by serializing it to the console, as identified by the global.ex() function by @WarInternal that I mentioned in a previous post. I use that in the console frequently to debug, and that was why I was and still am against that option. I am still a proponent of the console.log() inside of the .toJSON() function, warning about what they are doing. Player education over player coddling...

Gankdalf

One extra note is that RawMemory could be used to bypass this for people who actually know how it works, however, even if you delete/replace/destroy/whatever the old Memory object in global, the Creep.prototype.memory and other similar accessors seem to still reference the unparsed Memory and cause a second parse. These should probably be looked at before adding this.

SemperRabbit

@artch said in StructureSpawn causing "circular structure" error on JSON.stringify():

We can add RawMemory.autoCreateObjects boolean property which is true by default, but you can change it to false before accessing Memory first time on the tick.

I think it should be false by default, but if there were an option I can use to prevent this, I would be absolutely ecstatic.