search

oyachai / HearthSim

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

Card.deepCopy is currently huge performance hit #116

Closed slaymaker1907 closed 9 years ago

slaymaker1907 commented 9 years ago

As it is currently written, deepCopy uses reflection to obtain the constructor of the currently class which proves to be a gigantic performance hit. According to some rough profiling I did, approximately 13% of execution time is spent on a single line of code in this method : "copy =getClass().newInstance();".

Since reflection lookup as is currently done is roughly 10x as expensive as just instantiating it using the new operator (see http://stackoverflow.com/questions/435553/java-reflection-performance) we could probably achieve about a 10% speedup if we can figure out how to implement this without reflection so I definitely think that this is a worthwhile optimization if it can be done.

One idea that I've had would be to use some sort of lookup table to store constructors so that reflection happens only once as opposed to millions of time since it would be hard to provide the functionality of deepCopy without any reflection at all.

slaymaker1907 commented 9 years ago

I'm going to close this, because even when I used the .clone method, the performance was pretty much identical leading me to think that this optimization isn't really possible.