TickThreading icon indicating copy to clipboard operation
TickThreading copied to clipboard

Less ambitious, less rubbish 1.12 release

Open LunNova opened this issue 8 years ago • 62 comments

Tasks:

  • [x] Split javassist patching implementation into separate project
    • [x] Split non-minecraft code into JavaPatcher
    • [x] Split minecraft-specific code into ModPatcher
  • [x] Mixin implementation in nallar/JavaPatcher. nallar/Javapatcher#2
    • [x] Mixin implementation in Mixin nallar/Mixin#1
    • [x] Mixin transformations in ModPatcherGradle
  • [x] ~~Improve performance in ModPatcher by avoiding re-transforming~~ nallar/ModPatcher#10
  • [x] Implement old profiling features in separate mod. nallar/TickProfiler#22
  • [ ] Start from scratch, bring in some important features. Avoid feature creep
    • [x] Implement variant system (core/threaded for now, should be extensible)
    • [ ] Implement per-world threading
    • [ ] Implement deadlock detector
    • [ ] async/threaded chunkloading improvements
      • [ ] Fix the bloody stupid chunkloading when you world.get with coords an unloaded chunk
      • [ ] Chunkloading cache which actually works
    • [ ] Entity collision performance fixes
    • [x] Fix FMLCommonHandler.getEffectiveSide -> hardcode as always running on server
    • [ ] Avoid slow operations on TE/Entity lists. See TickManager.batchRemove(Tile)Entities
    • [ ] Fix poor World.getPersistentChunks performance

LunNova avatar Jan 07 '16 15:01 LunNova

Glad to hear you're coming back! I'm still sticking with your 1.5.2 release and fix everything myself :D.

MrVoltz avatar Jan 08 '16 20:01 MrVoltz

Wooow welcome back!!! We are so glad your coming whit 1.8 !!!!

Vycraftujto avatar Jan 09 '16 22:01 Vycraftujto

Sorry about the spam when I closed all the issues there.

I've closed all the old issues, as I won't be doing any further work on older MC versions.

LunNova avatar Jan 09 '16 23:01 LunNova

I've added a task list with basic plans for the first preview release. Have I missed any features people depend on?

I want to try to avoid adding too many features. TT ended up with a LOT of feature creep.

For example, in the new version TT will not have its own profiling, as the same features will be in TickProfiler. I'll need to work on improving TickProfiler's profiling features to match what used to be in TT.

LunNova avatar Jan 09 '16 23:01 LunNova

I'm not sure if you see a solution, anyways, the chunk unloading tends to be more intense than the load once the active/ticking tile entities hit +15k in the world. They are all stored in an array list and on chunk unload they are being removed. But to keep in mind, on tick update minecraft is iterating over the list, so other collections don't make sense either. On servers with +25k active tile ents we have run good with a list set combination. You gave us the idea about this some time back, not sure if you remember :) https://gist.github.com/Slind14/da64506db693f060afdd https://gist.github.com/Slind14/1531aa0d5db944cc61b0

What I could think of would be to run the chunkunloading only every n ticks for all chunks together, so that it only needs to go over the list once instead of for every chunk.

Anyways its all not about multi threading.

Regarding the collision fix, wouldn't make sense to be kept in separated mod, or would you use multithreading for it?

Slind14 avatar Jan 10 '16 08:01 Slind14

I'm not sure if you see a solution, anyways, the chunk unloading tends to be more intense than the load once the active/ticking tile entities hit +15k in the world.

Old TT did as few operations as possible against those lists, and batched any additions/removals to them to once per tick using addAll/removeAll, which is much more efficient than separate remove calls. See TickManager.batchRemove(Tile)Entities. Added to task list.

Regarding the collision fix, wouldn't make sense to be kept in separated mod, or would you use multithreading for it?

Fair point. I had this fix in old versions of TT as it's reasonably easy and helps performance a lot. Spigot sort of did the same thing, but not as well/aggressively. It's not really threading related though.

Reopened #107. The name currently isn't representative of what TT aims to do. Performance improvements, some but not all due to threading.

My "NTweaks" mod is mainly a rushed stop-gap measure because I was running a server for a few friends and there were approximately infinite squid spawning, and I couldn't change dimensions more than a few times due to memory leaks. I'm surprised it's been useful for other people. New TT will be using the same style of configurable patching as used there, so any unwanted patches can be turned off.

LunNova avatar Jan 10 '16 15:01 LunNova

Sounds good. My main concern regarding the collosion fix (and entity list batch remove) was that there might be users that don't want or can't use the multi threading part but would like to use this part. Kinda like nTweaks not being part of TT or ModPatcher.

Slind14 avatar Jan 10 '16 15:01 Slind14

I thought further about it and believe it would really be better if those non threading related and non critical patches would be in a separated mod. The performance impact of the collision patch and the entity list batch removes are a big improvement on their own. Especially the batch removes as those avoid the annoying lag spikes on base unload. From my understanding those wouldn't be too difficult to port. Hence they could probably be used way before the major multi threading is production ready and even on 1.7, if a back porting is manageable.

Slind14 avatar Jan 10 '16 20:01 Slind14

What about Optifine style core/extra/ultimate (probably got the names wrong) releases?

LunNova avatar Jan 10 '16 20:01 LunNova

Like?

  • basic (solid proof patches, very unlikely to interfere with mods)
  • extra (production ready patches, could be incompatible with some mods)
  • ultimate (experimental patches, likely to interfere with mods)

Slind14 avatar Jan 10 '16 20:01 Slind14

Yes, something like that.

  • <PlaceHolderName>-core
  • <PlaceHolderName>-threaded
  • <PlaceHolderName>-experimental

(TickThreading-threaded really doesn't work as a name)

LunNova avatar Jan 10 '16 21:01 LunNova

Do you want something functional (giving an idea of what it does) or creative (no relation, e.g. forge, bukkit, sponge, glowstone). Functional I could think of PerfOverhaul (performance overhaul). I'm not sure if that is a common word, in my native language it is used in terms of car and machine tuning.

Slind14 avatar Jan 11 '16 10:01 Slind14

Quick status update:

  1. Still working on Mixin - trying hard to avoid the mess the prepatcher was in previous versions
  2. Expect to start working on network profiling in TickProfiler by Sunday
  3. Vague estimate of starting work on TT proper within two weeks

Still looking for input on the name. I'll make a poll once there are enough good suggestions.

PerfOverhaul - very descriptive, but also quite generic. Not sure if I like it

LunNova avatar Jan 20 '16 15:01 LunNova

Ended up working in a slightly different order. Started the work on 1.8.9 TT first, mainly by deleting lots of classes and adding a single Mixin, just to confirm that ModPatcherGradle + ModPatcher + Mixin are working correctly.

Next thing to do is to add network profiling to TP.

My website/jenkins/maven repo may be down for some time due to a hosting migration during the weekend.

LunNova avatar Jan 28 '16 14:01 LunNova

Nallar please make a patreon so server owners can give tribute to your greatness :)

ProsperCraft avatar Jan 28 '16 22:01 ProsperCraft

Hosting migration is now complete.

Nallar please make a patreon

Maybe later, once it works.

LunNova avatar Feb 02 '16 13:02 LunNova

Status update:

TickProfiler for 1.7.10 and 1.8.9 both have support for all profliing options which TT used to have, except for the /ticks command which will continue to be TT only.

After far too much faffing around with classloading (maybe 4 hours?), including ModPatcher's API only in multiple mods of mine now works without conflicts and with automatic updating.

Now to start work on TT!

LunNova avatar Feb 08 '16 01:02 LunNova

~~Temporary downtime for nallar.me/jenkins starting in 5 minutes, lasting approximately 15 minutes while I swap the UPS the server is on.~~

Maintenance completed with no issues.

LunNova avatar Feb 13 '16 16:02 LunNova

Status update:

Done some work on TT. After starting to actually use the new mixin patching, I ran into some deficiencies mainly involving generics, so more work on JavaTransformer/Mixin was needed to get that working.

I won't be doing much if any work on this over the weekend, as I will be attending StrathHACK.

LunNova avatar Feb 19 '16 21:02 LunNova

Should have a testing release out by Friday evening GMT. It will not do any threading at all - only a few minor performance improvements.

Goal with this is to find any compatibility issues with the new patching system.

LunNova avatar Feb 23 '16 19:02 LunNova

1.8.9 only I suppose?

Slind14 avatar Feb 23 '16 19:02 Slind14

Yes

LunNova avatar Feb 23 '16 19:02 LunNova

Unfortunately I'm not able to offer testing for 1.8.9

Slind14 avatar Feb 24 '16 09:02 Slind14

@nallar Is the test release going to be out tonight?

XPownage avatar Feb 26 '16 23:02 XPownage

Ended up spending more time than expected working on a project for uni, so the testing release will probably be delayed until tomorrow. :(

I want to get the variants system (optifine style multiple release types) implemented as part of the testing release.

Sorry for the delay.

LunNova avatar Feb 27 '16 00:02 LunNova

@nallar Its alright, I'd rather want something that you are more conformable with releasing at the end of the day.

XPownage avatar Feb 27 '16 00:02 XPownage

First 1.8.9 testing release!. Grab the "-core" jar and drop it in your mods folder.

The aim is that it should do: absolutely nothing! A very minor performance improvement by hardcoding the current side in FMLCommonHandler, but nothing substantial.

Hoping to hear that it a) doesn't cause crashes b) has no noticeable impact. This is a test that the new patcher system isn't incompatible. The "-threaded" variant jar will break the game, that's expected.

LunNova avatar Feb 28 '16 02:02 LunNova

Awesome! I'll be more than glad to give you input on crashes and other related issues that I might find.

XPownage avatar Feb 28 '16 05:02 XPownage

@nallar Ok well I tired launching the core version and it just didn't work.
28.02 05:58:46 [Server] main/INFO [STDERR]: [java.lang.Throwable:printStackTrace:634]: Caused by: java.lang.ClassNotFoundException: me.nallar.modpatcher.ModPatcher$Version

XPownage avatar Feb 28 '16 05:02 XPownage

Oops - my test setup had the ModPatcher jar in the mods folder, so it didn't run into that problem. Should now be fixed.

LunNova avatar Feb 28 '16 11:02 LunNova