Pulse optimization - [merged] #178

Closed jdonkervliet closed 4 years ago

In GitLab by @swabbur on Jun 11, 2020, 02:42

Merges feature/optimizations -> development

Resolves issue #105 and #112.

The main gameloop was quite messy and filled with code duplication, I've tried to clean it up as much as I could, using the Stream API to allow us to process parts of the loop in parallel.

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

Commented on src/main/java/net/glowstone/GlowWorld.java line 709

Do we have any indication as to whether the JVM can optimize this the same/better than a normal for loop?

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

Commented on src/main/java/net/glowstone/GlowWorld.java line 709

I expect it to be harder to optimize, so finding an alternative would be crucial to ensure performance. Do you know a better way of doing this via the Stream API?

In GitLab by @swabbur on Jun 12, 2020, 01:31

Commented on src/main/java/net/glowstone/GlowWorld.java line 709

changed this line in version 2 of the diff

In GitLab by @swabbur on Jun 12, 2020, 01:31

added 1 commit

0f72eda5 - Move away from streaming only

Compare with previous version

In GitLab by @swabbur on Jun 12, 2020, 01:33

added 1 commit

0bd5de8a - Add TODO for entity spawning

Compare with previous version

In GitLab by @swabbur on Jun 12, 2020, 09:25

added 1 commit

b7c2741b - Remove unused imports

Compare with previous version

In GitLab by @swabbur on Jun 12, 2020, 09:40

added 1 commit

6f6a5214 - Do not broadcast updates for removed entities

Compare with previous version

In GitLab by @swabbur on Jun 12, 2020, 11:56

added 1 commit

ee19cc97 - Fix incorrect chunk iteration

Compare with previous version

In GitLab by @swabbur on Jun 13, 2020, 10:07

added 2 commits

cc0caaf4 - Broadcast spawn messages
882c9d7c - Broadcast block updates and clean up GlowWorld

Compare with previous version

In GitLab by @swabbur on Jun 15, 2020, 18:01

added 13 commits

882c9d7c...01be8b50 - 12 commits from branch development
97ef4567 - Merge branch 'development' into feature/optimizations

Compare with previous version

In GitLab by @swabbur on Jun 16, 2020, 17:48

added 2 commits

29a16ad9 - Revert entity spawn broadcasting
92ed0670 - Integrate and clean the AreaOfInterest

Compare with previous version

In GitLab by @swabbur on Jun 16, 2020, 18:02

added 1 commit

94604c8e - Fix CheckStyle errors

Compare with previous version

In GitLab by @JimVliet on Jun 16, 2020, 18:19

Commented on src/main/java/net/glowstone/GlowWorld.java line 761

The arguments should all be on different lines.

In GitLab by @JimVliet on Jun 16, 2020, 19:44

Commented on src/main/java/net/glowstone/chunk/ChunkIterator.java line 21

Might it be worth it to clone the area here, to ensure that mutating the area of interest does not break the iterator?

In GitLab by @JimVliet on Jun 16, 2020, 19:56

Commented on src/main/java/net/glowstone/GlowWorld.java line 709

Not really.

In GitLab by @swabbur on Jun 17, 2020, 14:12

Commented on src/main/java/net/glowstone/chunk/ChunkIterator.java line 21

The area class is immutable, all its fields are final. So, no copying is required anywhere.

In GitLab by @swabbur on Jun 17, 2020, 15:28

added 3 commits

7f448352 - Document the chunk iterator and spliterator
caba046b - Fix incorrectly refactored spawn method
48b12939 - Disable AreaOfInterest tests

Compare with previous version

In GitLab by @swabbur on Jun 17, 2020, 15:42

Commented on src/main/java/net/glowstone/GlowWorld.java line 761

changed this line in version 12 of the diff

In GitLab by @swabbur on Jun 17, 2020, 15:42

added 2 commits

cbecdf10 - Fix codestyling of changed code and remove update and broadcast measurements
3140ad7c - Rename PlayerFilter to FeedbackFilter

Compare with previous version

In GitLab by @swabbur on Jun 17, 2020, 16:57

added 1 commit

9dda695a - Test the AreaOfInterest class

Compare with previous version

In GitLab by @swabbur on Jun 17, 2020, 17:01

Commented on src/main/java/net/glowstone/GlowWorld.java line 761

I've fixed these lines. Must have been interrupted while refactoring it haha

In GitLab by @swabbur on Jun 17, 2020, 17:01

unmarked as a Work In Progress

In GitLab by @swabbur on Jun 17, 2020, 17:01

changed the description

In GitLab by @JimVliet on Jun 17, 2020, 17:02

resolved all threads

In GitLab by @larsdetombe on Jun 17, 2020, 18:27

I think you forgot to mention that this merge request also solves #112 right?

In GitLab by @larsdetombe on Jun 17, 2020, 18:31

Commented on src/main/java/net/glowstone/GlowWorld.java line 1677

Was this method unused? Or did we change other methods such that this method was not required anymore?

In GitLab by @larsdetombe on Jun 17, 2020, 18:33

Commented on src/main/java/net/glowstone/GlowWorld.java line 1687

Is the new Message[0] done on purpose? because before this was new Message[spawnMessage.size()]

In GitLab by @larsdetombe on Jun 17, 2020, 18:38

Was there a reason that the ChunkSpliterator and the ChunkIterator don't have a test class?

In GitLab by @swabbur on Jun 17, 2020, 18:59

Commented on src/main/java/net/glowstone/GlowWorld.java line 1677

The method was unused to begin with, but I know we had previously added it while working on the physics.

In GitLab by @swabbur on Jun 17, 2020, 19:02

Commented on src/main/java/net/glowstone/GlowWorld.java line 1687

Yes, passing an empty array to the toArray is a way to ensure that only one allocation happens (an array of size 0 doesn't allocate any heap memory, and the toArray will notice that the array size is too small and will therefore create his own array).

In this case, the original array would have been large enough, but doing it this way is more consistent with times where you aren't sure.

In GitLab by @swabbur on Jun 17, 2020, 19:03

Yes... I figured testing them would be a waste of time haha

I can still do so if you want me to, but I would prefer it to do it on a separate branch such that Julian can start experimenting.

In GitLab by @larsdetombe on Jun 17, 2020, 19:04

nah, was just curious tbh

In GitLab by @larsdetombe on Jun 17, 2020, 19:04

resolved all threads

In GitLab by @larsdetombe on Jun 17, 2020, 19:05

changed the description

In GitLab by @wubero on Jun 18, 2020, 12:03

merged

In GitLab by @wubero on Jun 18, 2020, 12:03

mentioned in commit 20c8373e6ae42666bbb8b7b9b3edb5e66713fdb3

atlarge-research / opencraft

Pulse optimization - [merged] #178