oakes / play-clj

A Clojure game library
The Unlicense
940 stars 72 forks source link

Fixes memory leak and input issue. #74

Closed brycecovert closed 9 years ago

brycecovert commented 9 years ago

The input issue is caused by the fact that the previous screen is still rendered, even after .setScreen is called. Executing on the gl thread fixes this.

The documentation says that old screens need to be disposed manually. This instantly freed up a lot of memory when running my game.

oakes commented 9 years ago

Wow! I was actually aware about dispose being manual, but I thought it didn't actually do anything unless you implemented the method yourself. Anyway, thanks.

brycecovert commented 9 years ago

You know what, I actually take that back. I don't think that's actually freeing up memory like I thought it was. I think maybe top wasn't updating. :(

Zach Oakes mailto:notifications@github.com March 7, 2015 at 8:54 AM

Wow! I was actually aware about |dispose| being manual, but I thought it didn't actually do anything unless you implemented the method yourself. Anyway, thanks.

— Reply to this email directly or view it on GitHub https://github.com/oakes/play-clj/pull/74#issuecomment-77697873.

oakes commented 9 years ago

Ah darn, I'll revert then. I wonder if the memory leak is caused by my use of intern. I heard that it can lead to leaks. On Mar 7, 2015 11:59 AM, "Bryce" notifications@github.com wrote:

You know what, I actually take that back. I don't think that's actually freeing up memory like I thought it was. I think maybe top wasn't updating. :(

Zach Oakes mailto:notifications@github.com March 7, 2015 at 8:54 AM

Wow! I was actually aware about |dispose| being manual, but I thought it didn't actually do anything unless you implemented the method yourself. Anyway, thanks.

Reply to this email directly or view it on GitHub https://github.com/oakes/play-clj/pull/74#issuecomment-77697873.

Reply to this email directly or view it on GitHub https://github.com/oakes/play-clj/pull/74#issuecomment-77698154.

brycecovert commented 9 years ago

Looking at this further, it seems like you are still in charge of disposing of textures and the like that are not in use (not just the screen). Wouldn't this cause leaks?

I'm thinking that the dispose implementation should delegate to each screen, to allow each to discard of any textures such.

Zach Oakes mailto:notifications@github.com March 7, 2015 at 9:02 AM Ah darn, I'll revert then. I wonder if the memory leak is caused by my use of intern. I heard that it can lead to leaks. On Mar 7, 2015 11:59 AM, "Bryce" notifications@github.com wrote:

You know what, I actually take that back. I don't think that's actually freeing up memory like I thought it was. I think maybe top wasn't updating. :(

Zach Oakes mailto:notifications@github.com March 7, 2015 at 8:54 AM

Wow! I was actually aware about |dispose| being manual, but I thought it didn't actually do anything unless you implemented the method yourself. Anyway, thanks.

Reply to this email directly or view it on GitHub https://github.com/oakes/play-clj/pull/74#issuecomment-77697873.

Reply to this email directly or view it on GitHub https://github.com/oakes/play-clj/pull/74#issuecomment-77698154.

— Reply to this email directly or view it on GitHub https://github.com/oakes/play-clj/pull/74#issuecomment-77698321.

oakes commented 9 years ago

If you aren't using an asset manager, that could cause a leak. However, the leak is even happening in Nightmod, which uses an asset manager and clears all assets when you restart a game.

On Sat, Mar 7, 2015 at 12:06 PM, Bryce notifications@github.com wrote:

Looking at this further, it seems like you are still in charge of disposing of textures and the like that are not in use (not just the screen). Wouldn't this cause leaks?

I'm thinking that the dispose implementation should delegate to each screen, to allow each to discard of any textures such.

Zach Oakes mailto:notifications@github.com March 7, 2015 at 9:02 AM Ah darn, I'll revert then. I wonder if the memory leak is caused by my use of intern. I heard that it can lead to leaks. On Mar 7, 2015 11:59 AM, "Bryce" notifications@github.com wrote:

You know what, I actually take that back. I don't think that's actually freeing up memory like I thought it was. I think maybe top wasn't updating. :(

Zach Oakes mailto:notifications@github.com March 7, 2015 at 8:54 AM

Wow! I was actually aware about |dispose| being manual, but I thought it didn't actually do anything unless you implemented the method yourself. Anyway, thanks.

Reply to this email directly or view it on GitHub https://github.com/oakes/play-clj/pull/74#issuecomment-77697873.

Reply to this email directly or view it on GitHub https://github.com/oakes/play-clj/pull/74#issuecomment-77698154.

Reply to this email directly or view it on GitHub https://github.com/oakes/play-clj/pull/74#issuecomment-77698321.

Reply to this email directly or view it on GitHub https://github.com/oakes/play-clj/pull/74#issuecomment-77698498.

brycecovert commented 9 years ago

Got it. Thanks!

Zach Oakes mailto:notifications@github.com March 7, 2015 at 9:10 AM If you aren't using an asset manager, that could cause a leak. However, the leak is even happening in Nightmod, which uses an asset manager and clears all assets when you restart a game.

On Sat, Mar 7, 2015 at 12:06 PM, Bryce notifications@github.com wrote:

Looking at this further, it seems like you are still in charge of disposing of textures and the like that are not in use (not just the screen). Wouldn't this cause leaks?

I'm thinking that the dispose implementation should delegate to each screen, to allow each to discard of any textures such.

Zach Oakes mailto:notifications@github.com March 7, 2015 at 9:02 AM Ah darn, I'll revert then. I wonder if the memory leak is caused by my use of intern. I heard that it can lead to leaks. On Mar 7, 2015 11:59 AM, "Bryce" notifications@github.com wrote:

You know what, I actually take that back. I don't think that's actually freeing up memory like I thought it was. I think maybe top wasn't updating. :(

Zach Oakes mailto:notifications@github.com March 7, 2015 at 8:54 AM

Wow! I was actually aware about |dispose| being manual, but I thought it didn't actually do anything unless you implemented the method yourself. Anyway, thanks.

Reply to this email directly or view it on GitHub https://github.com/oakes/play-clj/pull/74#issuecomment-77697873.

Reply to this email directly or view it on GitHub https://github.com/oakes/play-clj/pull/74#issuecomment-77698154.

Reply to this email directly or view it on GitHub https://github.com/oakes/play-clj/pull/74#issuecomment-77698321.

Reply to this email directly or view it on GitHub https://github.com/oakes/play-clj/pull/74#issuecomment-77698498.

— Reply to this email directly or view it on GitHub https://github.com/oakes/play-clj/pull/74#issuecomment-77698684.