Code COMMUNITYBOARD - RankingSystem (Top 15 PVP/PK)

Zake · August 11, 2021

1 hour ago, melron said:
-	public void showRakingList(Player player)
+	public synchronized void showRakingList(Player player)
@Zake ReentrantLock is a bad way to solve this 'problem'

Nope, it's better than synchronized keyword in such occasions. There is no reason putting in queue a bunch of requests when all you want to achieve is synchronizing a connection.

StinkyMadness · August 12, 2021

3 hours ago, melron said:

@StinkyMadness I would do the whole thing a bit different, like a manager which will storing the infos in custom classes and generate the content directly up on the player's request with a FloodProtector check. No sync at all

_nextUpdate is kinda FloodProtector.. but the way you talking about if I'm not wrong you saying each player can update once each 60 seconds... that's bad in server with players better adding ThreadPool each 60 seconds I don't see what's bad on my second pastebin I have posted.. but people that want to use that code can do the changes based on your recommendations.

melron · August 12, 2021

11 hours ago, Zake said:

Nope, it's better than synchronized keyword in such occasions. There is no reason putting in queue a bunch of requests when all you want to achieve is synchronizing a connection.

I would prefer to see some examples from your side by showing why you prefer that way instead of synchronize. To be honest, ive never used it in l2 scene but im using it a lot in my work. I'll show you an example. Lets say we have some dozens requests per minute from random users and the 'main' method that will be executed is the method named 'test'

private void test(User user)
{
	doSomething1(user);
	doSomething2(user);
	startUpdating(user); // here we need to prevent any other user to execute this method
	doSomething3(user);
	doSomething4(user);
	doSomething5(user);
	stopUpdating(user); // until this method take place. When that ends, we release the locked object.
	doSomething6(user);
	doSomething7(user);
}

As you can see, here, is a perfect example to use ReentrantLock just because we will start the lock case in 'startUpdating' and unlock in 'stopUpdating' while all the other users will stay at 'doSomething2' but also, the main user will continue on 3,4,5 methods and finally will be released after the stopUpdating method is fired. By using synchronize on the method structure or even with keyword and brackets {obj} , will cause big problems. We also using it just to lock something for a specific time periods. All i want to say is, its just 'too much' for this example.. Ofc it could work but again, is too much...

Quote

There is no reason putting in queue a bunch of requests

Actually this is exactly what ReentrantLock is doing with its own way, with the main difference that you can 'prevent' the infinitely synchronization of the sync keyword which in this example would never happen.

10 hours ago, StinkyMadness said:

_nextUpdate is kinda FloodProtector.. but the way you talking about if I'm not wrong you saying each player can update once each 60 seconds... that's bad in server with players better adding ThreadPool each 60 seconds I don't see what's bad on my second pastebin I have posted.. but people that want to use that code can do the changes based on your recommendations.

You can let it as it is, you wont have problems with that change of the 2nd pastebin. Nonetheless i said the same thing about the TP just in another manager. Im posting an optimized version of your code with my style with the difference that i used the same manager . Also, you dont have to care so much about the objects that have to be created up on the player's request just because it is java... GC is there...

https://pastebin.com/NukMRMgj

Edited August 12, 2021 by melron

Rootware · August 13, 2021

For all smart guys which gave pointless recommendations for improving @StinkyMadness code need a bit more learn an OOP concept.

This is an ideal solution from your paranoia with concurrent access:

String HTML_TEXT = "<html><body>Some Text</body></html>";

long TIMESTAMP = 0;
AtomicBoolean _underUpdate = false;

public void showMePage(Player player)
{
	if (TIMESTAMP < System.curentTimeMilliseconds() && !underUpdate)
	{
		_underUpdate = true;
		// get data from DB here, build HTML text and save into HTML_TEXT field.

		TIMESTAMP = System.curentTimeMilliseconds() + 60000L;
		_underUpdate = false;
	}

	player.sendHTML(HTML_TEXT);
}

Sorry guys for this words but your potential case possible if server will ran on Pentium 1. In other cases a server performance enough for fast updating statistic and caching it in class fields between few near requests.

melron · August 13, 2021

2 hours ago, Rootware said:
For all smart guys which gave pointless recommendations for improving @StinkyMadness code need a bit more learn an OOP concept.

This is an ideal solution from your paranoia with concurrent access:
String HTML_TEXT = "<html><body>Some Text</body></html>";

long TIMESTAMP = 0;
AtomicBoolean _underUpdate = false;

public void showMePage(Player player)
{
	if (TIMESTAMP < System.curentTimeMilliseconds() && !underUpdate)
	{
		_underUpdate = true;
		// get data from DB here, build HTML text and save into HTML_TEXT field.

		TIMESTAMP = System.curentTimeMilliseconds() + 60000L;
		_underUpdate = false;
	}

	player.sendHTML(HTML_TEXT);
}
Sorry guys for this words but your potential case possible if server will ran on Pentium 1. In other cases a server performance enough for fast updating statistic and caching it in class fields between few near requests.

Sorry root, but your words are not corresponding to your code. An OOP style is complete different than this code. Actually is the opposite.

In the other hand, i finding useless and pointless the atomic boolean. Explain me your thought up there, if you dont have that boolean and make it like this:

	if (TIMESTAMP < System.curentTimeMilliseconds())
	{
        	TIMESTAMP = System.curentTimeMilliseconds() + 60000L;
		// get data from DB here, build HTML text and save into HTML_TEXT field.
	}

What would be the problem? As i mentioned before and aggreed about the 2nd change of stinky's code, that would be a solution too. I personally, didn't want to be the smart guy with the better code if you see my previous posts, but tried to optimize the code with a different way of coding. instead of timestamps i preferred to use classes to do the job.

And about the performance you mentioned, we dont have to look the hardware and compare the speed, but the code itself.

btw, i would love to see your script completed and see how you will feed the 'string' but also, the way of 'resetting' that string. You tried to simplify it but you created another spaces

Rootware · August 13, 2021

@melronno problem. Explaning for ytou all fields.

String HTML_TEXT = "<html><body>Some Text</body></html>";

This field contains all dynamic data from page. It can be full page or only data grid part of rating.

long TIMESTAMP = 0;

This field need for detecting a fact of outdating a cache only. Why like this? It's simple. If no request from players, no requests for updates as long as possible.

AtomicBoolean _underUpdate = false;

This field need as thbread-safety flag about "caching process already started in another thread" for concurrent access cases. The first request with outddated cache state will use additional load for server w/o systematic actions via another separated threads. In time while cache rebuilding all other requests will use cached data from string.

In the end, this pattern do not contains resources more than need for handle this rank.

I named you and other speakers as "smart guys" because if you tries to teach someone to fix issues in code then make this job better as it possible. No need rework something in class if pattern of class is totally wrong.

The reason of my "anger" post was in it. You should evaluate how appropriate a minor fix is and suggest a different class pattern if it is more correct than fixing all potentially dangerous places of code. This is the major problem of all teachers in L2J (ofc with me).

P.S. I'm not suggested anything, by the way.

Tryskell · August 13, 2021

One threadpool task which caches the String from the db query would be enough - It doesn't need to be called below 1 hour, imho.

Nightw0lf · August 13, 2021

3 hours ago, Tryskell said:

One threadpool task which caches the String from the db query would be enough - It doesn't need to be called below 1 hour, imho.

thats what i said and my comment has been deleted and now zake likes your post aswell what an irony!

SweeTs · August 13, 2021

Umm, I didn't look into the code, just read some posts and it seems like the PLAYER, actually, is "triggering" the update? If so, that's plain stupid, lol. I would simply put the update under the thread pool and update list each X time. I see no reason for any timestamps and so on.. Thread pool updating itself. It could not be simpler.

Edited August 13, 2021 by SweeTs

Zake · August 13, 2021

9 minutes ago, Nightw0lf said:

thats what i said and my comment has been deleted and now zake likes your post aswell what an irony!

It's my responsibility as a moderator to clean spam replies, i'm doing this (almost) on a daily basis. Any opinion is welcome as long as you don't offend someone (not only me ofcourse). Also, if you scroll up you can see that ReentrantLock is my suggestion based on the author's approach, i never said that i would use this instead of threadpool. According to author's approach is the fastest thread-safe solution (although i do know that the difference is waaaaaaay too small). Last but not least,my intention on deleting your replies was not bad, i hope that you are not offended.

Nightw0lf · August 13, 2021

10 minutes ago, Zake said:

It's my responsibility as a moderator to clean spam replies, i'm doing this (almost) on a daily basis. Any opinion is welcome as long as you don't offend someone (not only me ofcourse). Also, if you scroll up you can see that ReentrantLock is my suggestion based on the author's approach, i never said that i would use this instead of threadpool. According to author's approach is the fastest thread-safe solution (although i do know that the difference is waaaaaaay too small). Last but not least,my intention on deleting your replies was not bad, i hope that you are not offended.

i am offended cause this was the second time you did that, the first was on support section, and i am thinking seriously leaving the forum since when i try to help some how you just remove my comments like you have personal issues with me.

PS show my comments to the letter so people can see what i've said excactly and you removed it

Zake · August 13, 2021

1 minute ago, Nightw0lf said:

i am offended cause this was the second time you did that, the first was on support section, and i am thinking seriously leaving the forum since when i try to help some how you just remove my comments like you have personal issues with me.

PS show my comments to the letter so people can see what i've said excactly and you removed it

As i said i am doing this very often, any of the moderators can confirm that. There is nothing personal on you or any other guy, i don't even know you actually, do i? Also, unfortunately i don't have access to deleted posts only Maxtor does, all i remember is one taylor swift gif and a meme for java classes or something. You are smart enough to understand that such comments should be avoided in share/contribute sections.

Nightw0lf · August 13, 2021

yes act like you dont know me too, if you would read my comment you would see that it was not a spam not the first (in help section) but also the second time (here), what you say is proving only that you remove random comments without reading them, my oppinion on leaving the forum is not changing from your replies you are making it worse.

StinkyMadness · August 14, 2021

Thanks for the offtopic.

//On topic

@SweeTs the only reason of timestamp is because statistic page its the most abandon page of CB.

In case of ThreadPool of 1-5 minutes it will do connection w/o reason.. as it will visited a lot of less times.

In case of ThreadPool of 1 hour.. then its not statistic its sh!t

With timestamp you avoid the ThreadPool and useless database connection each X time.

Kara · August 14, 2021

The lack of knowledge in this topic is monstrous.

True story, adding 1 thread worker and 1 database connection every so it's expensive in almost 2022 for a game server that opens a database connection on every item pick anyway and execute a thread worker on every spell cast or attack of each player. ROFL

@Nightw0lf Please en-light me about on how "heavy" is a 0.002 millisecond connection every 1 hour

Edited August 14, 2021 by Kara

Code COMMUNITYBOARD - RankingSystem (Top 15 PVP/PK)

Recommended Posts

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Join the conversation

Posts

Topics