Jump to content

Recommended Posts

Posted

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.

Posted

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

AnvilChunkLoader

.

Please don't PM me to ask for help. Asking your question in a public thread preserves it for people who are having the same problem in the future.

Posted

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.

 

I do Forge for free, however the servers to run it arn't free, so anything is appreciated.
Consider supporting the team on Patreon

Posted

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.

Posted

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.

I do Forge for free, however the servers to run it arn't free, so anything is appreciated.
Consider supporting the team on Patreon

Posted

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)?

Posted

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.

I do Forge for free, however the servers to run it arn't free, so anything is appreciated.
Consider supporting the team on Patreon

Posted

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.

Apparently I'm a complete and utter jerk and come to this forum just like to make fun of people, be confrontational, and make your personal life miserable.  If you think this is the case, JUST REPORT ME.  Otherwise you're just going to get reported when you reply to my posts and point it out, because odds are, I was trying to be nice.

 

Exception: If you do not understand Java, I WILL NOT HELP YOU and your thread will get locked.

 

DO NOT PM ME WITH PROBLEMS. No help will be given.

Join the conversation

You can post now and register later. If you have an account, sign in now to post with your account.
Note: Your post will require moderator approval before it will be visible.

Guest
Unfortunately, your content contains terms that we do not allow. Please edit your content to remove the highlighted words below.
Reply to this topic...

×   Pasted as rich text.   Restore formatting

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.

Announcements



×
×
  • Create New...

Important Information

By using this site, you agree to our Terms of Use.