Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Poor performance past 0.2.1 #40

Closed
TheComputerizer opened this issue Apr 22, 2022 · 4 comments · Fixed by #60
Closed

Poor performance past 0.2.1 #40

TheComputerizer opened this issue Apr 22, 2022 · 4 comments · Fixed by #60
Assignees
Labels
bug Something isn't working

Comments

@TheComputerizer
Copy link

TheComputerizer commented Apr 22, 2022

I really should have reported this a while back when I noticed that, but I have been using darkness-0.2.1-all for a while instead of the newer versions due to every version past 0.2.1 cutting my FPS in half.

I decided to do a direct FPS comparison between darkness-0.2.1-all, darkness-forge-1.12.x-0.4.2, and Hardcore Darkness and here are the results of that.

Unfortunately my FPS on latest version of this is still very bad.

Here is my modpack if you would like the list of mods. I am using the latest forge version of 14.23.5.2860

@TheComputerizer
Copy link
Author

I did some spark profiling for 0.2.1 and 0.4.2 and something weird is definitely going on.

Here is the 0.2.1 profile and here is the 0.4.2 profile

Whatever the method io.github.yamporg.darkness.asm.EntityRendererHooks.updateLuminance() is taking up over 50% of the client thread in 0.4.2 which would account for my FPS getting cut in half

@tie tie self-assigned this Nov 26, 2022
@tie tie added the bug Something isn't working label Nov 26, 2022
@tie tie added this to To do in Dynamic Darkness via automation Nov 26, 2022
@tie
Copy link
Member

tie commented Nov 27, 2022

Seems to be an issue with mod compatibility and hook injection logic. Currently the hook can potentially be injected at multiple points if other coremods insert net.minecraft.client.renderer.texture.DynamicTexture updateDynamicTexture calls in net.minecraft.client.renderer.EntityRenderer updateLightmap.

cr.accept(
new ClassVisitor(Opcodes.ASM5, cw) {
@Override
public MethodVisitor visitMethod(
int access,
String name,
String desc,
String signature,
String[] exceptions) {
MethodVisitor mv =
super.visitMethod(access, name, desc, signature, exceptions);
if (!UPDATE_LIGHTMAP_NAME.equals(name)
|| !UPDATE_LIGHTMAP_DESC.equals(desc)) {
return mv;
}
return new GeneratorAdapter(api, mv, access, name, desc) {
@Override
public void visitMethodInsn(
int opcode,
String owner,
String name,
String desc,
boolean itf) {
if (!itf
&& Opcodes.INVOKEVIRTUAL != opcode
&& !UPDATE_DYNAMIC_TEXTURE_OWNER.equals(owner)
&& !UPDATE_DYNAMIC_TEXTURE_NAME.equals(name)
&& !UPDATE_DYNAMIC_TEXTURE_DESC.equals(desc)) {
loadThis();
loadArgs();
invokeStatic(
Type.getObjectType(ENTITY_RENDERER_HOOKS),
new Method(
ON_UPDATE_LIGHTMAP_NAME,
ON_UPDATE_LIGHTMAP_DESC));
}
super.visitMethodInsn(opcode, owner, name, desc, itf);
}
};
}
},
ClassReader.SKIP_FRAMES);

tie added a commit to tie/mods that referenced this issue Nov 27, 2022
Fixes the incorrect hook injection target. Before this change, the hook was
injected at every method instruction causing noticeable performance degradation.

Fixes yamporg#40
@tie tie closed this as completed in #60 Nov 27, 2022
tie added a commit that referenced this issue Nov 27, 2022
Fixes the incorrect hook injection target. Before this change, the hook was
injected at every method instruction causing noticeable performance degradation.

Fixes #40
Dynamic Darkness automation moved this from To do to Done Nov 27, 2022
@tie
Copy link
Member

tie commented Nov 27, 2022

@TheComputerizer I've published v0.5.0 release with the fix, let me know if the issue is still relevant for you.

@KxttyKxt
Copy link

Hey @tie:

I have had this issue in the past as well. In my testing with darkness-forge-1.12.x-0.5.0.jar, I have found the issue is no longer relevant. My FPS (when set to unlimited) is upwards of 140, as opposed to 20 fps like it was before.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Development

Successfully merging a pull request may close this issue.

3 participants