Implements a cache for train collision checks for better performance #9513

stacode123 · 2025-11-08T14:19:47Z

Implements a cache for train collision checks for better performance
Before: https://spark.lucko.me/cgjtsUl25f
After: https://spark.lucko.me/p7ZcDxAUFA
Criticism welcome.
Made with the help of Copilot

World used for testing :
world.zip

…e networks

sleepy-evelyn · 2025-11-08T18:07:23Z

If this works properly this is brilliant @stacode123 and definately needed. We've turned train collisions off entirely to help with lag but thats definately not a fix most servers want to implement.

Using Copilot is a little worrying though because you need to know exactly whats going on for this. Make sure you've tested it thoroughly and used the debugger to step through the code to see whats going on and if it aligns with what you expect.

Jozufozu

Very excited about this and the spark profile is extremely promising. A 5mspt difference is huge

How many trains did you use in your test world? It would be helpful to upload a screenshot of the chaos 😆

Also after the requested changes and before this merges I'd like to request a longer profile to compare. I'm a little nervous the 16 second profiles may not have enough signal to noise

src/main/java/com/simibubi/create/content/trains/entity/Train.java

Jozufozu · 2025-11-08T21:13:02Z

src/main/java/com/simibubi/create/content/trains/entity/Train.java

+				collisionCache.addSegment(lastPoint, start, segmentIndex++);
+			}
+
+			if (segmentIndex < totalSegments) {


How will segmentIndex ever be greater or equal to totalSegments? Same for above

Again, do we need these checks? They could hide critical bugs for the same reason I described here: #9513 (comment)

src/main/java/com/simibubi/create/content/trains/entity/Train.java

Jozufozu · 2025-11-08T21:21:04Z

src/main/java/com/simibubi/create/content/trains/entity/Train.java

+			if (length > 0.0001) {
+				direction[index] = diff.normalize();
+			} else {
+				direction[index] = Vec3.ZERO;


How will this happen?

I see you've removed this but please justify it

stacode123 · 2025-11-08T21:36:24Z

test world has 180 trains more than that would have caused my computer to have a seizure

Will address the comments

stacode123 · 2025-11-09T11:27:43Z

Performance impact on a server with 475 trains:

Before:

After:

sleepy-evelyn · 2025-11-09T16:07:33Z

To be honest it's rare u see such a stark contrast between before and after. A similar number of players are connected to the server as well so the difference likely isn't down to that. :)

…llisionCache calls.

stacode123 · 2025-11-09T19:04:32Z

@Jozufozu Please review the changes :D

Jozufozu · 2025-11-09T19:46:44Z

src/main/java/com/simibubi/create/content/trains/entity/Train.java


-		Vec3 start = (speed < 0 ? trailingPoint : leadingPoint).getPosition(graph);
-		Vec3 end = (speed < 0 ? leadingPoint : trailingPoint).getPosition(graph);
+		Vec3 start = collisionCache.start[0];


Where did the speed < 0 checks go? If the train is moving backwards we need to check for collisions with the trailing end as if it were the head

Oh also do we know for sure that the collision cache is non-null here?

That is handled when collision cache is created.

Earlier in the train tick we updated the collision cache but i can add a null check just to be sure

Jozufozu · 2025-11-09T19:47:54Z

src/main/java/com/simibubi/create/content/trains/entity/Train.java

-
 		Vec3 normedDiff = diff.normalize();
-		double maxDistanceSqr = Math.pow(AllConfigs.server().trains.maxAssemblyLength.get(), 2.0);
+		double maxDistanceSqr = AllConfigs.server().trains.maxAssemblyLength.get()*AllConfigs.server().trains.maxAssemblyLength.get();


Please only fetch the config value once. This call showed up in your spark profile already and I do not want to rely on the JVM to deduplicate these.
Also formatting

double maxDistance = AllConfigs.server().trains.maxAssemblyLength.get(); double maxDistanceSqr = maxDistance * maxDistance;

Jozufozu · 2025-11-09T19:48:50Z

src/main/java/com/simibubi/create/content/trains/entity/Train.java

-
-			// Use cached data if available, otherwise fall back to old method
-			if (train.collisionCache != null && train.collisionCache.isValid()) {
+			if (train.collisionCache != null ) {


nit invert the check and early continue to save on indentation

Jozufozu · 2025-11-09T19:50:42Z

src/main/java/com/simibubi/create/content/trains/entity/Train.java

+			if (length > 0.0001) {
+				direction[index] = diff.normalize();
+			} else {
+				direction[index] = Vec3.ZERO;


I see you've removed this but please justify it

Jozufozu · 2025-11-09T19:51:38Z

src/main/java/com/simibubi/create/content/trains/entity/Train.java

+				collisionCache.addSegment(lastPoint, start, segmentIndex++);
+			}
+
+			if (segmentIndex < totalSegments) {


Again, do we need these checks? They could hide critical bugs for the same reason I described here: #9513 (comment)

Jozufozu · 2025-11-09T19:52:41Z

src/main/java/com/simibubi/create/content/trains/entity/Train.java

+		if(segmentIndex == 0) {
+			collisionCache=null;
+		}


nit formatting

if (segmentIndex == 0) { collisionCache = null; }

I dont know why it was there in the first place

ok ill remove them

fixed

Jozufozu · 2025-11-09T19:54:26Z

src/main/java/com/simibubi/create/content/trains/GlobalRailwayManager.java

+		for (Train train : movingTrains){
+			train.updateCollisionCache();
+			train.earlyTick(level);}


formatting please

for (Train train : movingTrains) { train.updateCollisionCache(); train.earlyTick(level); }

do we not need the collision cache for waiting trains? do we not expect moving trains to collide with trains that are stationary?

Wont stationary trains have already built up cache ? And trying to calculate it again would just yield the same result?

What about on initial level load? Can we really assume that a train that just became stationary this tick will have valid positions in the cache from the previous tick where it was moving?

Still, that's a good point. If you're feeling ambition you could probably check some easy to compute value at the start of updateCollisionCache to see if the train has moved from the previous tick and decide whether to update the collision cache. If you do though, I'd like to see proof that it works and waiting trains still get collided with

Jozufozu · 2025-11-09T19:54:57Z

src/main/java/com/simibubi/create/content/trains/entity/Train.java

 		if (derailed || graph == null) {
 			if (collisionCache != null)
-				collisionCache.invalidate();
+				collisionCache = null;


probably don't need the collisionCache != null check anymore. just assigning is fine

what do you mean by that

Jozufozu · 2025-11-09T19:55:27Z

src/main/java/com/simibubi/create/content/trains/entity/Train.java

+			collisionCache = new CollisionCache(totalSegments);
+		}
+
+		Vec3 lastPoint = null;


Add a comment in the code please

Jozufozu · 2025-11-09T19:55:36Z

src/main/java/com/simibubi/create/content/trains/entity/Train.java

+	private void updateCollisionCache() {
+		if (derailed || graph == null) {
+			if (collisionCache != null)
+				collisionCache.invalidate();


Add a comment in the code please

stacode123 added 2 commits November 8, 2025 15:15

Implement a cache for train collisions for better performance on larg…

f18968c

…e networks

Remove redundant fallback

3f80853

Jozufozu self-requested a review November 8, 2025 21:19

Jozufozu requested changes Nov 8, 2025

View reviewed changes

Address comments and further improve performance by reducing updateCo…

7e58290

…llisionCache calls.

Jozufozu requested changes Nov 9, 2025

View reviewed changes

Address comments

956dcc1

stacode123 requested a review from Jozufozu November 16, 2025 12:48

Implements a cache for train collision checks for better performance #9513

Are you sure you want to change the base?

Implements a cache for train collision checks for better performance #9513

Conversation

stacode123 commented Nov 8, 2025

Uh oh!

sleepy-evelyn commented Nov 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Jozufozu left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stacode123 commented Nov 8, 2025

Uh oh!

stacode123 commented Nov 9, 2025

Uh oh!

sleepy-evelyn commented Nov 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

stacode123 commented Nov 9, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Jozufozu Nov 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

sleepy-evelyn commented Nov 8, 2025 •

edited

Loading

sleepy-evelyn commented Nov 9, 2025 •

edited

Loading

Jozufozu Nov 9, 2025 •

edited

Loading