oyachai / HearthSim

Generic Hearthstone game simulator
MIT License
314 stars 59 forks source link

Gigantic boost to concurrency scalability as well as fixes and improvements for DeckFactory #115

Closed slaymaker1907 closed 9 years ago

slaymaker1907 commented 9 years ago

As it was before this update, the ImplementedCardList.getInstance method was causing a tremendous slowdown when using many threads to run simulations due to the way it implemented the singleton pattern since the getInstance method had to be synchronized since it might initialize the singleton.

This update changes the way the singleton works by initializing the singleton upon class loading by using a static block and thereby ensuring thread safety without synchronization.

I've done some benchmarking, and this seems to have drastically increased the scalability of running concurrent simulations. On a computation server with 24 cores, it seemed to barely be able to utilize 4 cores (corresponding to 400% using a the top command in Linux) and with the change, it now is using about 20 cores (corresponding to 2000%). On my local 8 core machine, I also saw a more modest but still significant speedup.

The change to DeckFactory fixes the filterByRarity method to actually work and not throw exceptions as well as adding a feature to allow the builder methods to easily be chained together.

oyachai commented 9 years ago

Hi, this looks like a great change, but it is failing on travis. Could you please check and update the PR?

slaymaker1907 commented 9 years ago

The issue was a rare existing bug in the test code but should be fixed now.

oyachai commented 9 years ago

good stuff. Thanks!