[1.7.10] Question on AnvilChunkLoader....

[OreCruncher]

I was looking at the writeChunkToNBT() code in order to understand how the save data is generated.  I noticed that a bunch of byte arrays are stuffed into the NBT save object.  Looking underneath the NBT code, all it will do is wrap the byte buffer without copying.  The net result is that the byte buffer is now shared between the NBT "persisted" state and the Chunk object.  If that Chunk object updates the byte array before that NBT gets written to disk it will magically update in the NBT state which will no longer make that state consistent, and the results recorded into the RegionFile cannot be reliable.

 

Is this something to worry about?  Or is the shared state an OK thing?

 

An underlying question I have is whether an NBTByteArray object can be considered immutable, like a String.

[OreCruncher]

I would assume the same would apply to the NBTTagIntArray as well.  Thanks for the response.

 

BTW, does Forge modify AnvilChunkLoader, or does that come straight from Vanilla?

[Choonster]

You can see the changes Forge makes to vanilla classes in the patches folder of the GitHub repository. This is the patch for

AnvilChunkLoader

.

[OreCruncher]

Do I need to open a ticket or will it be mystically handled by the powers of the Forge devs?

[LexManos]

1) 1.7.10 is old update

2) Yes this MIGHT be able to cause some slight desync issues depending on what happens in the world and how long it takes for your to save the chunk. However as long as you're not having disk issues {in which case there is going to be a lot more issues unrelated to this} there shouldn't be an issue. You'd just have updated block data.

 

Do you have an actual issue this is causing?

Duplicating those chunk arrays would be pretty problematic as thats a lot of array allocation/copying.

 

[OreCruncher]

Do you have an actual issue this is causing?

 

No, I don't.  Found the condition via code inspection.  If this problem were to occur it could manifest in some subtle ways so it would be hard to spot.  Other thing is that the saved data could work itself out - a subsequent save would be consistent.  I would have to wonder, though, about a read of the NBT while the chunk data is waiting to be written (cache hit).  Not sure of all the conditions that could occur - I just know that it does happen.

 

Duplicating those chunk arrays would be pretty problematic as thats a lot of array allocation/copying.

 

Yes, the performance impact would be significant.  Not only do the buffers up in AnvilChunkLoader have to be duplicated, the NBT tree would have to be crawled looking for other buffer type tags and do the duplicate as well.  Reason is that the tags for modded items aren't aware of what is happening down in IO land.

 

I will have to rely on your experience about the seriousness of the issue.  As you point out the conditions would be rare, and if it does occur the impact wouldn't be destabilizing.

 

EDIT:  Did a quick wiff test as to the performance impact.  What I did was introduce a "freeze" method in the NBT class hierarchy that will cause a tag to duplicate it's buffer when invoked, and is smart enough to figure out when it doesn't have to do it (i.e. NBTTagList of NBTTagInts don't need to be frozen since the ints are immutable at this stage).  The allocated NBT structure will traverse the tree as needed to perform this deed.  With the freeze, writeChunkToNBT() took on average 35ms, and without the free it took about 25ms to complete.  The freeze process takes 40% more time.

[LexManos]

Ya, that 40% increase isn't taking into account memory either. The only real issue this would popup is POTENTIALLY duplicate entities as they traverse the chunk boundary between the chuck queue and actual save. {TEs have a similar issue}

However, things *should* be non-ticking in 99% of all these cases so that shouldn't be an issue.

So on the extremely low probability that this is a issue MC has mechanics in place to sanitize things on load so the world state should be fine.

[OreCruncher]

I have a further question related to your statement about the time between a given chunks saves and desyncs.  What I am wondering is if multiple IO threads were indeed operating it would be possible that two different IO threads could wind up with two different states for a given chunk at the same time assuming the writes to disks were horribly tardy.  Would be safe to put this situation in the same category as the NBT buffer sharing (i.e. stop worrying about it and buy lottery tickets because the chance of hitting the jackpot are greater than running into this race condition)?

[LexManos]

There are two possible threads doing IO work, the Write{Thread dedicated to just saving chunks} and the Read{Main Server Thread}

The Read thread is smart enough to check the Write thread's queue and read the chunk from there if it isn't written yet.

So there is a very minute chance that issues may arise cuz Threading is always fun, but its got safeguards and sanitization for that.

[OreCruncher]

Threading is indeed fun.  It's one of the aspects of development I enjoy.  Also like databases, but I don't say that too loudly.   Databases are like threads in some ways.

[Draco18s]

Threading is indeed fun.  It's one of the aspects of development I enjoy.  Also like databases, but I don't say that too loudly.   Databases are like threads in some ways.

 

No, not really. It's way easier to destroy a project that has a database.