Discussion Reason of delay?

[Ο Χάρος]

What would be the reason of pause the tread of player for like 1.5 to 2 seconds upon sendSkillList? 

I did berchmark on the

public void sendSkillList(final L2PcInstance player)

code it return literally 0.06 millisecond respond time So thats no the reason. There is no other code

both admin (give_all_skills) and the sub-class use the sendSkillList(player);  which it really has no delay since i made it's List and it's really fast.

What else would cause this 1.5 - 2 second delay upon give skill list?  Give some advice

[Elfo]

It's client side. The server is bombarding the client with one packet per skill and it essentially gets flooded.

[Ο Χάρος]

21 minutes ago, .Elfocrash said:

It's client side. The server is bombarding the client with one packet per skill and it essentially gets flooded.

Wait wait.. the skillList is a packet that contains a sub-list so basically  on a List you load all getPlayerAvailableSkills 

and u add them on SkillList packet and then at the end u send 1 time the packet.  so ?  It's not about fix. 1.5 sec delay is no problem is just

it make me so fucking curious.. i did berchmarks and is so fast that even in nanoseconds it show 3500 to complete all giveSkillListToPlayer(); 

Edited September 8, 2017 by Ο Χάρος

[Tryskell]

The method to retrieve skills is probably not optimized. The same effect happens on subclass acquisition, where you can wait a solid 3 to 5 seconds, only because the method doesn't filter correctly things, and reward you will all possible skills (and reward skills means one database query per skill acquisition, because there is no global method to reward all skills), while you should filter first and reward after.

sendSkillList is probably ok and got no specific reason to bug by itself (there was a slight edit to do for a specific case, but otherwise nothing to do).

Before holidays on Croatia, I was actually reworking the whole thing on aCis.

 

You should add more details about which methods you are invoking, what you try to do, etc.

Edited September 8, 2017 by Tryskell

[Elfo]

19 minutes ago, Tryskell said:

The method to retrieve skills is probably not optimized. The same effect happens on subclass acquisition, where you can wait a solid 3 to 5 seconds, only because the method doesn't filter correctly things, and reward you will all possible skills (and reward skills means one database query per skill acquisition, because there is no global method to reward all skills), while you should filter first and reward after.

sendSkillList is probably ok and got no specific reason to bug by itself (there was a slight edit to do for a specific case, but otherwise nothing to do).

Before holidays on Croatia, I was actually reworking the whole thing on aCis.

 

You should add more details about which methods you are invoking, what you try to do, etc.

EDIT

sendSkillList() is packing the skills together but the restoreSkills() method is locking the shit out of the thread because of the sql involved.

Edited September 8, 2017 by .Elfocrash

[Ο Χάρος]

Let me get the things straight:

This is the admin method that give ALl skills to players.  and the method that does that is the sendSkillList ( same called in sub-class)

	private void adminGiveAllSkills(final L2PcInstance activeChar, final boolean includedByFs)
	{
		final L2Object target = activeChar.getTarget();
		L2PcInstance player = null;
		if (target instanceof L2PcInstance)
		{
			player = (L2PcInstance) target;
		}
		else
		{
			activeChar.sendPacket(SystemMessage.getSystemMessage(SystemMessageId.INCORRECT_TARGET));
			return;
		}
		player.sendSkillList();
	}

 

Now in this method as u can see: 

public void sendSkillList(final L2PcInstance player)
	{
		boolean isDisabled = false;
		final SkillList sl = new SkillList();
		
		if (player != null)
		{
			for (final L2Skill s : player.getAllSkills())
			{
				if (s == null)
				{
					continue;
				}
				if (s.getId() > 9000 && s.getId() < 9007)
				{
					continue;
				}
				if (_transformation != null && !containsAllowedTransformSkill(s.getId()) && !s.allowOnTransform())
				{
					continue;
				}
				if (player.getClan() != null)
				{
					isDisabled = s.isClanSkill() && player.getClan().getReputationScore() < 0;
				}
				boolean isEnchantable = SkillTable.getInstance().isEnchantable(s.getId());
				if (isEnchantable)
				{
					final L2EnchantSkillLearn esl = EnchantGroupsTable.getInstance().getSkillEnchantmentBySkillId(s.getId());
					if (esl != null)
					{
						if (s.getLevel() < esl.getBaseLevel())
						{
							isEnchantable = false;
						}
					}
					else
					{
						isEnchantable = false;
					}
				}
				if (getAioEndTime() > System.currentTimeMillis() && !isGM() && !isInsideZone(ZONE_TOWN))
				{
					isDisabled = true;
				}
				sl.addSkill(s.getId(), s.getLevel(), s.isPassive(), isDisabled, isEnchantable);
			}
		}
		
		sendPacket(sl);
	}

 

All skills are gathered into a packet that is send only 1 time upon the end of code. This class takes 3500 nanoseconds to be done, aka 0 milliseconds

 

The only method that retrieve the skills here is just this:

 

public final L2Skill[] getAllSkills()
    {
        if (_skills == null)
        {
            return new L2Skill[0];
        }
        return _skills.values().toArray(new L2Skill[_skills.values().size()]);
    }

Basically a Map that is turned into array (no delay at all obviously). 

 

I did in both methods 

long ms = System.getNanoTime(); 

System.out.println(System.getNanoTime() - ms); and it show all 0 MS in total.  MAX 1 ms. So the reason of 2 sec delay is not justified

[Ο Χάρος]

6 minutes ago, .Elfocrash said:

He said the code returns the skills in milliseconds but the client lags. If the method returns data instantly then it is clearly the client.

If it wasn't then every player would lag not just the one.

 

I did compare to H5 branch (cause i use freya) and the code is the same (just in H5 they added 1 more check for add the skill on holders) nothing to do with retreive or store. 

But in H5 it really takes 0.2 sec delay to give 45 skills while in freya it take 1.5 sec delay. I can imagine with extra skills it could take 3 sec delay.

[Elfo]

1 minute ago, Ο Χάρος said:

 

I did compare to H5 branch (cause i use freya) and the code is the same (just in H5 they added 1 more check for add the skill on holders) nothing to do with retreive or store. 

But in H5 it really takes 0.2 sec delay to give 45 skills while in freya it take 1.5 sec delay. I can imagine with extra skills it could take 3 sec delay.

It might come down to the packet length which they might have improved in H5.

[Ο Χάρος]

Just now, .Elfocrash said:

It might come down to the packet length which they might have improved in H5.

 

oh, true didn't think of that.  If i do   packet.toString().lenght();  this would return the packet size (somehow) i could compare to H5 then ?

[Elfo]

a

Edited October 1, 2021 by Elfocrash

[Ο Χάρος]

Just now, .Elfocrash said:

Nah the size should be roughly the same. It is the client code that might process it differently in H5

So basically you say that the size wouldnt change maybe the method that handle the packet is wrong. i check the H5 method SKilLIst and is this https://pastebin.com/AXws0Pgb

while on freya ishttps://pastebin.com/3HzmwR49

So basically on Freya the constructor create a new FastList which is slow as fuck compare to arrayList so ill go ahead and change this cause javolution is slower by 30% as i saw

I'll try my lack with this and report back. 

[Elfo]

a

Edited October 1, 2021 by Elfocrash

[Ο Χάρος]

Just now, .Elfocrash said:

There is no way that this is the problem.

Another obvious pointer would be that H5 is using HikariCP as the default db connection pool while Freya is still c3p0 which is like a million times slower (if somehow db is getting involved).

Now im thinking it's stupid cause the method was including in berchmark so any sub-calculations wouldnt again be recorded on system out at sendSkillList.. again i tested it nothing changed.. 

You're right..  No there is no DB connection involved.. I double check the code.

 

It goes from this: 

/**
	 * This function will give all the skills that the target can learn at his/her level
	 * @param activeChar: the gm char
	 */
	private void adminGiveAllSkills(final L2PcInstance activeChar, final boolean includedByFs)
	{
		final L2Object target = activeChar.getTarget();
		L2PcInstance player = null;
		if (target instanceof L2PcInstance)
		{
			player = (L2PcInstance) target;
		}
		else
		{
			activeChar.sendPacket(SystemMessage.getSystemMessage(SystemMessageId.INCORRECT_TARGET));
			return;
		}
		//Notify player and admin
		activeChar.sendMessage("You gave " + player.giveAvailableSkills(includedByFs, true) + " skills to " + player.getName());
		player.sendSkillList();
	}

To this 

 

public void sendSkillList()
	{
		sendSkillList(this);
	}

 

To this 

 

public void sendSkillList(final L2PcInstance player)
	{
		boolean isDisabled = false;
		final SkillList sl = new SkillList();
		
		if (player != null)
		{
			for (final L2Skill s : player.getAllSkills())
			{
				if (s == null)
				{
					continue;
				}
				if (s.getId() > 9000 && s.getId() < 9007)
				{
					continue;
				}
				if (_transformation != null && !containsAllowedTransformSkill(s.getId()) && !s.allowOnTransform())
				{
					continue;
				}
				if (player.getClan() != null)
				{
					isDisabled = s.isClanSkill() && player.getClan().getReputationScore() < 0;
				}
				boolean isEnchantable = SkillTable.getInstance().isEnchantable(s.getId());
				if (isEnchantable)
				{
					final L2EnchantSkillLearn esl = EnchantGroupsTable.getInstance().getSkillEnchantmentBySkillId(s.getId());
					if (esl != null)
					{
						if (s.getLevel() < esl.getBaseLevel())
						{
							isEnchantable = false;
						}
					}
					else
					{
						isEnchantable = false;
					}
				}
				if (getAioEndTime() > System.currentTimeMillis() && !isGM() && !isInsideZone(ZONE_TOWN))
				{
					isDisabled = true;
				}
				sl.addSkill(s.getId(), s.getLevel(), s.isPassive(), isDisabled, isEnchantable);
			}
		}
		
		sendPacket(sl);
	}

 

And stops. I use the admin command admin_give_all_skills  So is not SQL relative. Even if it was i have fast disk it wouldnt downgrade so much.

 

[Elfo]

Whats in 

player.getAllSkills()

[Ο Χάρος]

Just now, .Elfocrash said:

Whats in 

player.getAllSkills()

public final L2Skill[] getAllSkills()
    {
        if (_skills == null)
        {
            return new L2Skill[0];
        }
        return _skills.values().toArray(new L2Skill[_skills.values().size()]);
    }

Map to array