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

feat(Core/Threading): replace ace threading #4821

Merged
merged 20 commits into from
Apr 16, 2021

Conversation

Winfidonarleyan
Copy link
Member

@Winfidonarleyan Winfidonarleyan commented Mar 12, 2021

Changes Proposed:

  • Replace ACE threading

Issues Addressed:

Tests Performed:

  • Windows 3 people
    image
  • In Unix 2 people

How to Test the Changes:

  • Check enter in game (priority hight)
  • Uptime > 5-10 min

Target Branch(es):

  • Master

How to Test AzerothCore PRs

When a PR is ready to be tested, it will be marked as [WAITING TO BE TESTED].

You can help by testing PRs and writing your feedback here in the PR's page on GitHub. Follow the instructions here:

http://www.azerothcore.org/wiki/How-to-test-a-PR

@ghost
Copy link

ghost commented Mar 12, 2021

Apparently on Linux std::mutex works just fine but on Windows has some issues?

more about it here
https://stackoverflow.com/questions/14191566/c-mutex-in-namespace-std-does-not-name-a-type

@Winfidonarleyan
Copy link
Member Author

No problem in Win

@Mitradis
Copy link
Contributor

ready?

@Winfidonarleyan
Copy link
Member Author

In win maybe yes

@Mitradis
Copy link
Contributor

Compiling and work, but crashes on ctrl+c still happened.

@Mitradis
Copy link
Contributor

Mitradis commented Mar 15, 2021

Ok we can merge this i think, after linux user testing.

@Winfidonarleyan
Copy link
Member Author

Winfidonarleyan commented Mar 15, 2021

Okay, totaly replaced

@Winfidonarleyan
Copy link
Member Author

Ready

@Mitradis
Copy link
Contributor

this Pr should help with ctrlc crash? or this problem belongs to another part Ace remove?

Tested, work.

@Winfidonarleyan
Copy link
Member Author

this Pr should help with ctrlc crash? or this problem belongs to another part Ace remove?

No help with CTRL+C. Replace thread only

@FrancescoBorzi
Copy link
Contributor

@azerothcore/testers @azerothcore/developers @mpfans @pklloveyou please check this PR

@ghost
Copy link

ghost commented Mar 16, 2021

Tested on Windows 10

Overall no errors or weird behaviour was detected on a 13 minutes uptime server.

  • Played a bit around character creation and adding a few items using commands for a class.
  • Learnt spells and fought a Gorilla in Scholazar Basin
  • Tamed Loque'naraq
  • Uptime at the screenshot: 13 minutes

image

@FrancescoBorzi
Copy link
Contributor

good job @Winfidonarleyan and thank you very much @claudiodfc and @Mitradis for testing this.

As this is quite a big change, I'd like to have more people test this before merging.

@Zoidwaffle
Copy link
Contributor

@Winfidonarleyan : I'm recompiling your latest version now 👍

@Zoidwaffle
Copy link
Contributor

Zoidwaffle commented Mar 25, 2021

AzerothCore rev. c23fc113f999 2021-03-25 21:33:42 +0700 (mutex branch) (Unix, Release) uptime just passed 40 minutes, which is a new record. I will update later in the evening if it keeps working or not.

Edit: Uptime 2h10m. I'm starting to feel confident it's working now.

@Zoidwaffle
Copy link
Contributor

Zoidwaffle commented Mar 25, 2021

AC>AzerothCore rev. c23fc113f999 2021-03-25 21:33:42 +0700 (mutex branch) (Unix, Release)
Connected players: 0. Characters in world: 0.
Connection peak: 1.
Server uptime: 4 hour(s) 1 minute(s) 43 second(s).

Yep, it seems good. Well done! 🚀

Edit: Will compile and test with my modules etc. enabled as well.

Edit2: Seems fine, uptime ~2 hours now. I had to fix some SQL for a module, but that was all.

@Winfidonarleyan
Copy link
Member Author

So, all okay?

@Zoidwaffle
Copy link
Contributor

@Winfidonarleyan : yes, it seems to work just fine for me now. Thanks!

@FrancescoBorzi
Copy link
Contributor

@Winfidonarleyan the build is failing

@Winfidonarleyan
Copy link
Member Author

@Winfidonarleyan the build is failing

ElunaLuaEngine/Eluna#348

Copy link
Member

@Yehonal Yehonal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GJ! Have Someone tested it on a live server with medium population?

@FrancescoBorzi
Copy link
Contributor

waiting for @Footman @mpfans and @pklloveyou to test this with players

@mpfans
Copy link
Contributor

mpfans commented Apr 16, 2021

I've been testing it for a week and I think we can merge it

@FrancescoBorzi FrancescoBorzi merged commit b2861be into azerothcore:master Apr 16, 2021
@Winfidonarleyan Winfidonarleyan deleted the mutex branch April 16, 2021 19:22
@Winfidonarleyan
Copy link
Member Author

@mpfans Thanks for testing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Core/Threading: Rewrite Threading system [$150 awarded]
7 participants