AlchemicalWizardryTickHandler cpu leak
Slind14 opened this issue · 18 comments
Hey,
your AlchemicalWizardryTickHandler leaks massive cpu usage:
http://i.imgur.com/EwGutOu.png
What would happen if we comment it out ?
Would it break something major ?
mhh, that would mean that the effect would last for ever, right ?
I think it would be enough if you set it to 0.1 every 20th tick and only on players with the effect. If the effect lasts 1 second longer who cares. As the effect is permanent wouldn´t it be possible to only cast it when the effect gets removed and on playerJoin ?
Hi, if that would be irreversible, couldnt you just reset players with a sigil of haste? Or only reset in the tick the players loses the 'sigil of haste'-effect?
You'd still have to look if the player has that sigil, which might be worse. And what if they drop the sigil? What if they get the buff from another source?
If that line is commented out, the Boost effect from the Sigil of Haste would... be irreversible and blow up. Thaumcraft does something similar, however Azanor may have a way that is a lot better.
Would it be possible to save a list of players with active sigils? If a player is tested, you check if the Sigil is still there and if not (=dropped) you remove the buff. As only a few have the sigil, the total work would be less (I guess).
yeah + every 20th tick. This would lower down the cpu usage by about +20000% for us.
100 * 20 * 10 (if every 10th player uses a sigil)
Unfortunately that wouldn't exactly solve it (the one every 20th tick just can't work out). I think the issue is that the way it currently works also affects FakePlayers, which includes a bunch of blocks in the game (this is just a guess, but I don't know if it's actually the case). I'll think on it a bit and hope that I can find a solution that works out.
After browsing the code a bit, would it work to add something along the following to SigilOfHaste.java:
public ItemStack onItemRightClick(ItemStack par1ItemStack, World par2World, EntityPlayer par3EntityPlayer)
{
//...
tag.setBoolean("isActive", !(tag.getBoolean("isActive")));
//here comes my codeproposal
if(tag.getBoolean("isActive"))
{
activePlayers.add(par3EntityPlayer); //activePlayers would be a list which is checked in AlchemicalWizardryTickHandler
}
else
{
activePlayers.remove(par3EntityPlayer);
}
//here does it end
//...
}
On a server with 70 people with only 5 having the Sigil of Haste active, the extra work created by checking a list 65 times would be minimal compared to 65 unnecessary setPrivateAttribute-calls. Would of course extrawork if it would be reversed (eg. 65 having the sigil active)...
Maybe a Set would be better?
Would this be threadsafe? Please tell me, what do you think of this.
Unfortunately that wouldn't work, although it may be close to what I need. I'll have a hashmap save the names of the people using my mod's speed-altering items/buffs, and on the tick end I'll instead iterate over that list instead of all of the players. It should also remove the player from the list at each tick end, and depending on how well it works through testing, should constantly reset and re-add the player to the list.
Will it be more efficient per player? Probably not. Overall, though? I hope so! I'll do a bit of testing on this later on, and will close & comment this issue if it works.
@WayofTime come on IRC plox, I have a solution for this.
whats the solution now ? We just removed the check and it works fine. I don´t see any need for it on a big server.
@WayofTime Was this fixed?
It will be fixed in the 1.7.2 version for sure, and if I get it working there I may be able to port the fix to 1.6.4.
Please do, this is a major issue for us and would be a great fix to avoid disabling the item.
@WayofTime Status report? :P
@tterrag1098 Should be all fixed already!