Skip to content
This repository has been archived by the owner on Jun 19, 2021. It is now read-only.

C2ME Port #471

Open
wants to merge 11 commits into
base: staging/1.16.5
Choose a base branch
from
Open

C2ME Port #471

wants to merge 11 commits into from

Conversation

Titaniumtown
Copy link
Collaborator

@Titaniumtown Titaniumtown commented Apr 22, 2021

(supersedes #375)

Here are some performance numbers: (retested by ishland)

JVM Flags: -Xms12G -Xmx12G -XX:+UseG1GC -XX:+ParallelRefProcEnabled -XX:MaxGCPauseMillis=200 -XX:+UnlockExperimentalVMOptions -XX:+AlwaysPreTouch -XX:G1NewSizePercent=30 -XX:G1MaxNewSizePercent=40 -XX:G1HeapRegionSize=8M -XX:G1ReservePercent=20 -XX:G1HeapWastePercent=5 -XX:G1MixedGCCountTarget=4 -XX:InitiatingHeapOccupancyPercent=15 -XX:G1MixedGCLiveThresholdPercent=90 -XX:G1RSetUpdatingPauseTimePercent=5 -XX:SurvivorRatio=32 -XX:MaxTenuringThreshold=1 -Ddebug.rewriteForIde=true -Dusing.aikars.flags=https://mcflags.emc.gs -Daikars.new.flags=true
JVM: GraalVM 21.0.0.2 with Java11

ver/1.16.5 (main branch) (radius: 2048):

commit: 8d157f9be64fba5adb8d202417b3f8c80adb2c5a

  • 96.56 cps

C2ME Port (parallelism: 8 allow-threaded-features: true) (radius: 4096):

commit: 2ee8c246065be9980d5484ed561d7a3c0ec07a45

  • 565.95 cps

C2ME Port (parallelism: 8 allow-threaded-features: false) (radius: 4096):

commit: 2ee8c246065be9980d5484ed561d7a3c0ec07a45

  • 319.77cps

Old Threaded Worldgen Branch* (2 featuregen threads, 6 worldgen threads) (radius: 4096):

commit: 8e6a62ab90c9afeaeee2f49cca26f5ce264acb6d

  • 373.29 cps

*This PR is way more safer than Threaded Worldgen, fixing a lot of the issues people reported with #375.

@Titaniumtown
Copy link
Collaborator Author

Just a note: Java 11 or higher is required to run this PR as C2ME takes advantage of features only found in java 11 or higher. Since 49.4% (at the time of writing this comment) of servers use java 8, some decisions are going to have to be made on how to proceed with this PR.

@Titaniumtown
Copy link
Collaborator Author

Ok, the port is complete and everything is in place. A decision just needs to be made on how we're going to handle the new java version requirement.

@foss-mc
Copy link
Contributor

foss-mc commented May 3, 2021

Ok, the port is complete and everything is in place. A decision just needs to be made on how we're going to handle the new java version requirement.

A possible solution is to port it to java8 and remove the patch when Yatopia is updated to 1.17

From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: foss-mc <69294560+foss-mc@users.noreply.github.com>
Date: Mon, 3 May 2021 03:24:03 +0000
Subject: [PATCH] port AsyncCombinedLock to java8


diff --git a/src/main/java/org/yatopiamc/c2me/commom/util/AsyncCombinedLock.java b/src/main/java/org/yatopiamc/c2me/commom/util/AsyncCombinedLock.java
index b20fa65c0b6cf349d02e613aaefff426b840ae50..f4b66241c84206e49f812cd6ec5f2fdf788fd8c2 100644
--- a/src/main/java/org/yatopiamc/c2me/commom/util/AsyncCombinedLock.java
+++ b/src/main/java/org/yatopiamc/c2me/commom/util/AsyncCombinedLock.java
@@ -24,7 +24,7 @@ public class AsyncCombinedLock {
     private final CompletableFuture<AsyncLock.LockToken> future = new CompletableFuture<>();
 
     public AsyncCombinedLock(Set<AsyncLock> lockHandles) {
-        this.lockHandles = Set.copyOf(lockHandles);
+        this.lockHandles = new java.util.HashSet(lockHandles);
         lockWorker.execute(this::tryAcquire);
     }
 
@@ -34,18 +34,18 @@ public class AsyncCombinedLock {
         for (AsyncLock lockHandle : lockHandles) {
             final LockEntry entry = new LockEntry(lockHandle, lockHandle.tryLock());
             tryLocks.add(entry);
-            if (entry.lockToken.isEmpty()) {
+            if (!entry.lockToken.isPresent()) {
                 allAcquired = false;
                 break;
             }
         }
         if (allAcquired) {
-            future.complete(new CombinedLockToken(tryLocks.stream().flatMap(lockEntry -> lockEntry.lockToken.stream()).collect(Collectors.toUnmodifiableSet())));
+            future.complete(new CombinedLockToken(tryLocks.stream().filter(lockEntry -> lockEntry.lockToken.isPresent()).map(lockEntry -> lockEntry.lockToken.get()).collect(Collectors.toSet())));
         } else {
             boolean triedRelock = false;
             for (LockEntry entry : tryLocks) {
                 entry.lockToken.ifPresent(AsyncLock.LockToken::releaseLock);
-                if (!triedRelock && entry.lockToken.isEmpty()) {
+                if (!triedRelock && !entry.lockToken.isPresent()) {
                     entry.lock.acquireLock().thenCompose(lockToken -> {
                         lockToken.releaseLock();
                         return CompletableFuture.runAsync(this::tryAcquire, lockWorker);
@@ -80,7 +80,7 @@ public class AsyncCombinedLock {
         private final Set<AsyncLock.LockToken> delegates;
 
         private CombinedLockToken(Set<AsyncLock.LockToken> delegates) {
-            this.delegates = Set.copyOf(delegates);
+            this.delegates = delegates;
         }
 
         @Override

Copy link

@josephworks josephworks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. I'll do testing soon.

@josephworks
Copy link

Also, we could use java 11, but if java 8 is detected, it will download java 11 and use that.

@Titaniumtown
Copy link
Collaborator Author

cea262e actually added java 8 compat, but it failed to build recently (which I just now noticed) so I reverted that commit in b1d79de.

This reverts commit 6e5e95f.
This patch was unnecessary and caused issues in rare cases with blocks not being sent to the client properly. We've ruled out all other patches that could've possibly caused this issue, so this patch was the culperate.
Copy link

@josephworks josephworks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code works, if we are going to migrate to Java 11

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants