How to get the server world from a chunk generator

[Slit_bodmod]

Hi, sorry small topic but I am writing a custom chunk generator and in order for the generation to work properly I need to access some data stored in world saved data, and thus I need a serverWorld object, at first I starting by casting the IWorld object I am handed in the function I need it for (fillFromNoise aka func_230352_b_) into a ServerWorld and using that, it didn't work, so clearly I must be getting some different world object in the chunk generator.

My question is which world object am I getting and how do I convert or access it's associated ServerWorld object safely?

[Draco18s]

Chunk generation happens on a separate thread, so the IWorld is all you're going to get. You cannot access the main server world yet, both because (a) it might not exist and (b) it wouldn't be thread safe.

[Slit_bodmod]

Ah that makes a lot of sense, thankyou, so what would you recommend in order for me to get an instance of my worldSavedData object in time for chunk loading to begin? is there some way I can make sure that the chunk generator has an instance of this object before the process begins or some way to reach across in a safe manor?

[Slit_bodmod]

well what kind of alternatives are there for having level saveable data that my chunk generator can use, is there a way for me to pass other parameters into the constructor. Or a way for me to send data of any kind across the two threads. Maybe by saving a copy of the data statically somewhere and setting up appropriate checks around it?

Edited June 14, 2021 by Slit_bodmod

[Draco18s]

7 hours ago, diesieben07 said:

This is untested - but should work.

I read that like I read this:

import sun.java.misc.unsafe

[Slit_bodmod]

ok, thankyou that's very helpful. I have begun work implementing it and was wondering if I could ask a few questions?

- Would the values of my map also want to be weak as I presumably am also storing in the way that WorldSavedData automatically does so I don't want a copy of it to be stored

-Will the reference in my map remain a reference, there is a good chance that the main thread will change the contents of my WorldSavedData and Ideally the chunkGenerator should respond to these changes appropriately

- I don't know much about the internal structure of the jvm threading but seeing as I can guarantee that my ChunkGenerator won't be writing to the WorldSavedData, only reading, is there any chance of the multiple threads causing issues in this case

Quote

22 minutes ago, Draco18s said:

I read that like I read this:

import sun.java.misc.unsafe

 

-yes, would utilising the unsafe class be a good idea and how best can I go about it

-Also purely out of interest, would this system have been easier if I'd used capablities or mixins?

[Draco18s]

I was absolutely joking. The unsafe package is called "unsafe" because it is not safe to use.

[Slit_bodmod]

Hi sorry for late response,

Ok so I should create a map<World,WorldSavedData> that when a World Capability is attached will thread lock, and add a mapping of that world to it's appropriate savedData

Then in my chunk generator, using IWorld.getLevel() (on an instance ofc), I can look it up in the map and modify it at will, and if I ever do need to write to it for whatever reason I need to lock it first.

If this is true what system would you recommend to keep the variables safe? Should I use a java Lock class for the functionality I stated above or make the map volatile or reading data from the map synchronised?

[Slit_bodmod]

ok so would it work if I just put and get objects from my map as though everything was normal. and then once I've got my reference I need to write a synchronised method that will pull the data I need and return it to the chunk generator. (I'm only gonna need to pull one object so I think it would it be better for the synchronised function and return a deep copy?)

Also how do I get the ServerWorld from the IWorld, there isn't a function defined called getLevel, nor any function that returns a ServerWorld as far as I can see.

[Slit_bodmod]

ok so obviously I have a line of code that says this

public static final Map<World,TardisManager> INSTANCES = new MapMaker().weakKeys().makeMap();

then I have my init function that looks like this

@SubscribeEvent public static void onWorldLoading(AttachCapabilitiesEvent<World> event) {
        World world = event.getObject();
        DWMCMod.LOGGER.error("attaching capabilities"+world);
        if (world instanceof ServerWorld) {
            ServerWorld accessWorld = world.getServer().getWorld(DWMCDimensions.TARDIS_WORLD_KEY);
            TardisManager tardisManager = getTardisManagerForWorld(accessWorld);
            INSTANCES.put(accessWorld,tardisManager);
        }
    }

as you can probably guess, TardisManager is the WorldSavedData, it's basically a wrapper for a map that stores instances of Tardis which is kinda simple value struct, it's pretty much entirely primitive types (although it does contain an enum I'm going to have to pull an attribute from)

my only function that is synchronised thus, it is the one that pulls the tardis from the tardis manager

public synchronized TardisManager.Tardis getThreadedTardisForPos(IWorld world, BlockPos pos) {
        if (world instanceof IServerWorld) {
            ServerWorld server = ((IServerWorld)world).getWorld();
            TardisManager manager = INSTANCES.get(server);
            return new TardisManager.Tardis(manager.getTardisForBlock(pos));
        }
        DWMCMod.LOGGER.error("this shouldn't not be a server world I hope");
        return null;
    }

this is what that looks like, again, as you can probably guess, the TardisManager.Tardis constructor that takes in another TardisManager.Tardis is basically a copy constructor. it gets only once at the start of the fillFromNoise method in the chunk and the object it returns (that hopefully shouldn't interfere with other threads).

I think that's everything right?

[Slit_bodmod]

Oh also that enum I was talking about. So the attribute I need from that is a structure Template stored as a const variable within each instance of an enum. It thus only gets written to when the class is first inited and doesn't get edited beyond that so is that thread safe?

just for clarity, it's an enum that a reference to is stored in TardisManager.Tardis containing a constant field of type Template

[Slit_bodmod]

ah right, so the Tardis manager contains a few different get functions for different Tardises, will they all need to be synchronised, will this impact performance as it's only this one niece case where they need to be accessed by another thread.

getTardisForBlock returns a TardisManager.Tardis. the constructor that takes for TardisManager.Tardis that takes in another TardisManager.Tardis is basically just a copy constructor that returns a new object with the same values for this thread to modify and read.

in-fact the Tardis manager stores all instances of Tardis that the program ever uses inside of a map, all constructors for the Tardis are default accessibility because of this (except for the copy constructor which is public). If I made this map using "new MapMaker().makeMap()" would that help maybe.

As for TardisManager.Tardis, not all fields are mutable, including some I need for the chunk generator can be changed in code, however I don't really want these values changing during the generation of a given chunk atleast for the Tardis object that it's working on (which is why I made the function return a new copy)

Essentially would it make sense for me to have some synchronised function(or block in a function) that takes the Tardis from the original map and copies it in a thread safe manor.

Would it help if I sent the TardisManager.Tardis file, it's quite long as it also contains some relevant maths and networking functions so I could just send the necessary excerpts?

[Slit_bodmod]

right, well the trouble with creating a copy on the main thread is that the data is going to be modified as the world is running, particularly from the overworld (seeing as this chunk generator is designed to work for a specific dimension). so ideally I need the data to be the most up to date the moment I create a new chunk. That being said if the data changes while the chunk is being generated (or preferably when the region is being generated), the main thread still needs to keep track of that but I don't want the chunk generator to have its copy changed.

So getting a hold of the TardisManager is done through a thread safe map atm. Getting a reference to the specific tardis required is currently done from a non-thread safe map so can I just make it a thread safe map and that will be fine.

If so then real difficulty seems to be then getting all of the data from the Tardis object that map holds a reference too. As discussed the best way to do that is to try and create a copy of it in a thread safe manor for use by the chunk generator. The question obviously is how. I presume then synchronised won't help me out here as I can't make every function that writes into this data structure so I need to somehow lock the whole datastructure for writing at the start of the copy function and unlock it at the end, how do you think I might do this?

If this is the case should I remove synchronised from getThreadedTardisForPos function?

 

[Slit_bodmod]

On 6/19/2021 at 8:03 AM, diesieben07 said:

Does it access a non-thread-safe data structure (such as a HashMap) which is accessed from more than on thread?

No, I think if I make the underlying data structure in my TardisManager using a MapMaker it will be fine.

As for using the ReentrantReadWriteLock appropriately. I assume I should lock it at the beginning of the copy constructor and unlock it at the end. What impact will this have. when I read/write to the other fields in my data structure, I assume it won't automatically be safe, do they have lock it first and then write to the data strucuture?

[Slit_bodmod]

8 minutes ago, diesieben07 said:

Also note that "just use ConcurrentHashMap" will not magically make all your threading problems go away.

yes but will it stop me getting thread issues when reading and writing to that hash map?

12 minutes ago, diesieben07 said:

To guard a data structure using it you have to write-lock every time anyone writes to it and read-lock every time anyone reads from it.

right, I gather the best way to go about doing this is with getters and setters for every variable (aaargh), and also locking and unlocking the read locks at the start of all functions, does this include the constructors, I assume not. I assume I do at the start of my serialisation function.

also am I right to assume that the syntax for the locks is:

my_class_lock.readLock().lock()

my_class_lock.readLock().unlock()

my_class_lock.writeLock().lock()

my_class_lock.writeLock().unlock()

and I use the read locks for reading, write locks for writing, lock at the start and unlock at the end

[Slit_bodmod]

Yeah maybe it would be a better idea to have a separate immutable struct inside of my TardisManager.Tardis called TardisGenerationInfo or something that was always written in a thread safe way and was the only thing that the chunk generator was allowed to read from.

So the reference to tardisManager is gotten through a ConcurrentMap, as well as the reference to the specific Tardis. so maybe this new object is stored in a separate hash map, or a reference to it can only be used from within the Tardis data structure and contains basically an immutable structure that be accesed by the chunk generator and if any of the data updates, a lock is clicked denying read access, the reference is replaced with a new instance of TardisGenerationInfo and then the lock is unlocked.

actually if I made a system like this would I need to make it all immutable fields each read and write into this class was dependent on the lock.

[Slit_bodmod]

Ok so I the version I am currently using looks a bit like this.

I still have something that looks like this in order to pull a TardisManager.

public static final Map<World,TardisManager> INSTANCES = new MapMaker().weakKeys().makeMap();

and the TardisManager contains this map.

private final ConcurrentHashMap<Integer,Tardis> ALL_TARDISES = new ConcurrentHashMap();

so I can then pull a tardis object, I then have some stuff that looks like this inside of the Tardis class

private ReentrantReadWriteLock tardis_lock;
public InteriorStyle int_style;

public InteriorStyle getIntStyle() {
	tardis_lock.readLock().lock();
	InteriorStyle ret_style = int_style;
	tardis_lock.readLock().unlock();
	return ret_style;
}
public void setIntStyle(InteriorStyle new_style) {
    tardis_lock.writeLock().lock();
    int_style = new_style;
    tardis_lock.writeLock().unlock();
}

and that returns an InteriorStyle object which is an enum so all fields are final.

Is that what I'm supposed to do?

[Slit_bodmod]

Ok cool, I changed it to something like this:

public InteriorStyle getIntStyle() {
  try {
    tardis_lock.readLock().lock();
    InteriorStyle ret_style = int_style;
    return ret_style;
  } finally {
    tardis_lock.readLock().unlock();
  }
}

if there is any lock you would recommend over ReentrantReadWriteLock I'd be happy to use that, other wise you have been super helpful and I am very grateful