Concurrent chunk generation - [merged] #174

Closed jdonkervliet closed 4 years ago

In GitLab by @swabbur on Jun 6, 2020, 22:09

Merges feature/concurrent-chunk-generation -> development

Resolves issue #92.

In the current version of this branch, the chunks are being generated one after another, but via the priority executor. Allowing the main thread to continue processing messages and events, increasing the tick rate while loading many chunks.

In GitLab by @larsdetombe on Jun 8, 2020, 11:30

Commented on src/main/java/net/glowstone/chunk/ChunkManager.java line 112

Do you want to add a chunk here? Even though the original code is only meant for checking if the chunk is loaded and did not have the ability to add a chunk?

In GitLab by @larsdetombe on Jun 8, 2020, 11:33

Commented on src/main/java/net/glowstone/chunk/ChunkManager.java line 160

Can't this be caught in the upper try block, I believe it is possible to have multiple catches for one try statement. Becuase now we have a nested try-block

In GitLab by @JimVliet on Jun 8, 2020, 12:02

Commented on src/main/java/net/glowstone/chunk/ChunkManager.java line 500

This curly bracket should be on a newline.

In GitLab by @JimVliet on Jun 8, 2020, 12:05

Commented on src/main/java/net/glowstone/chunk/GlowChunk.java line 900

Perhaps we can determine the initial capacity based on the server view distance? I think that would make more sense than these seemingly random variables.

In GitLab by @swabbur on Jun 8, 2020, 20:17

Commented on src/main/java/net/glowstone/chunk/ChunkManager.java line 112

Adding a chunk here is trivial, as no loading is initiated, and would make sure that future calls made on the chunk variable always return the correct value.

In GitLab by @swabbur on Jun 8, 2020, 20:20

Commented on src/main/java/net/glowstone/chunk/GlowChunk.java line 900

As the map is initialized prior to the call to the main method, the server's view distance is not yet known. Increasing the reserved memory of the map after initializing the server seems a bit overkill, as it will most likely be resized many times during execution anyways.

In GitLab by @JimVliet on Jun 9, 2020, 24:19

Commented on src/main/java/net/glowstone/chunk/ChunkManager.java line 500

changed this line in version 2 of the diff

In GitLab by @JimVliet on Jun 9, 2020, 24:19

added 1 commit

35acd005 - Fix checkstyle errors for chunk generation

Compare with previous version

In GitLab by @JimVliet on Jun 9, 2020, 12:35

Commented on src/main/java/net/glowstone/chunk/ChunkManager.java line 160

If we catch it at the outer try/catch block, the server will not continue generating the chunk in case of a disk I/O error.

In GitLab by @JimVliet on Jun 9, 2020, 17:59

added 1 commit

df3832a5 - Move chunk generation to ChunkRunnable

Compare with previous version

In GitLab by @JimVliet on Jun 9, 2020, 18:07

added 12 commits

df3832a5...74f5eabd - 6 commits from branch development
c4692aa9 - Offload chunk generation to separate threads
9ac5c7d1 - Fix incorrectly ordered world initialization
8627b7c9 - Clean up chunk manager
c4138990 - Fix loop in populate chunk
66e08b8f - Fix checkstyle errors for chunk generation
77af4ac2 - Move chunk generation to ChunkRunnable

Compare with previous version

In GitLab by @JimVliet on Jun 9, 2020, 18:13

resolved all threads

In GitLab by @JimVliet on Jun 9, 2020, 18:13

unmarked as a Work In Progress

In GitLab by @swabbur on Jun 9, 2020, 18:21

Commented on src/main/java/net/glowstone/chunk/ChunkManager.java line 39

Why is the ChunkManager no longer final?

In GitLab by @swabbur on Jun 9, 2020, 18:24

Commented on src/test/java/net/glowstone/executor/PriorityRunnableTest.java line 46

As this class was created twice, might it be better to simply create a separate file for it? Something like TestRunnable?

In GitLab by @JimVliet on Jun 9, 2020, 19:58

Commented on src/main/java/net/glowstone/chunk/ChunkManager.java line 39

This was needed for mocking it in the ChunkRunnableTest class.

In GitLab by @JimVliet on Jun 9, 2020, 19:59

Commented on src/test/java/net/glowstone/executor/PriorityRunnableTest.java line 46

The name of the class is the same, but the implementation is not. So they need to be separate anyway.

In GitLab by @JimVliet on Jun 9, 2020, 20:11

added 1 commit

4eb96ae2 - Add a newline to end of PriorityRunnableTest

Compare with previous version

In GitLab by @swabbur on Jun 10, 2020, 10:23

resolved all threads

In GitLab by @JimVliet on Jun 10, 2020, 10:47

Commented on src/main/java/net/glowstone/chunk/ChunkManager.java line 39

changed this line in version 6 of the diff

In GitLab by @JimVliet on Jun 10, 2020, 10:47

added 1 commit

4a1dd3be - Enable mocking for the final ChunkManager class

Compare with previous version

In GitLab by @swabbur on Jun 10, 2020, 11:01

Commented on src/main/java/net/glowstone/executor/ChunkRunnable.java line 74

Missing whiteline at start of method.

In GitLab by @julian9499 on Jun 10, 2020, 11:01

Commented on src/main/java/net/glowstone/chunk/GlowChunk.java line 900

Can we make constants of these numbers? They don't mean much to me like this

In GitLab by @swabbur on Jun 10, 2020, 11:03

Commented on src/main/java/net/glowstone/executor/PriorityExecutor.java line 14

Having the generic type named identical to an explicit type might be a little confusing, is there an alternative name we could use?

In GitLab by @JimVliet on Jun 10, 2020, 11:32

Commented on src/main/java/net/glowstone/executor/PriorityExecutor.java line 14

changed this line in version 7 of the diff

In GitLab by @JimVliet on Jun 10, 2020, 11:32

added 1 commit

efb50107 - Rename generic PriorityRunnable and fix powermock

Compare with previous version

In GitLab by @JimVliet on Jun 10, 2020, 11:36

Commented on src/main/java/net/glowstone/chunk/GlowChunk.java line 900

changed this line in version 8 of the diff

In GitLab by @JimVliet on Jun 10, 2020, 11:36

Commented on src/main/java/net/glowstone/chunk/GlowChunk.java line 900

changed this line in version 8 of the diff

In GitLab by @JimVliet on Jun 10, 2020, 11:36

added 1 commit

48699fa0 - Add final variables for key hashmap config

Compare with previous version

In GitLab by @swabbur on Jun 10, 2020, 12:24

resolved all threads

In GitLab by @swabbur on Jun 10, 2020, 12:24

merged

In GitLab by @swabbur on Jun 10, 2020, 12:24

mentioned in commit 2601aced46ed3068d20fc722fcbc1d3eaed4496b

In GitLab by @JimVliet on Jun 10, 2020, 12:37

resolved all threads

atlarge-research / opencraft

Concurrent chunk generation - [merged] #174