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

pull request about issue #2062 (car_racing.py memory leaking issue ) #2096

Merged
merged 2 commits into from
Feb 16, 2021

Conversation

jackyoung96
Copy link
Contributor

@jackyoung96 jackyoung96 commented Nov 17, 2020

Pull request about issue #2062

There is memory leaking issue caused by pyglet module and solved with simply delete after drawing vertexs

I confirmed with e-mail from @praveenVnktsh

Copy link

@SwapnilPande SwapnilPande left a comment

Choose a reason for hiding this comment

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

Hi @Jaekyung-Cho,

I've been debugging the same issue. Looks like we need to add vl.delete() after vl.draw in function render_indicators (line 592). It looks like the memory consumption is still growing after this change, but much much slower.

@jackyoung96
Copy link
Contributor Author

Hi @Jaekyung-Cho,

I've been debugging the same issue. Looks like we need to add vl.delete() after vl.draw in function render_indicators (line 592). It looks like the memory consumption is still growing after this change, but much much slower.

Exactly. Memory leaking still remain, but I cannot find where it occur. Though slower leaking issues doesn't matter in my case.

@SwapnilPande
Copy link

Looks like this is an issue with Box2D, I was able to isolate the memory leak down to creating the tiles in _create_track. Despite calling DestroyBody on all of the tiles in _destroy, Box2D does not properly destroy the tiles sometimes. This memory leak can be reproduced with this minimum code:

import Box2D
from Box2D.b2 import fixtureDef, polygonShape

import psutil
import os

def memory_used():
    return psutil.Process(os.getpid()).memory_info().rss * 1e-6  # To megabyte


world = Box2D.b2World((0,0))


road1_l = (0, 0)
road1_r = (0, 1)
road2_l = (1, 0)
road2_r = (1, 1)
vertices = [road1_l, road1_r, road2_r, road2_l]

fd_tile = fixtureDef(
                shape = polygonShape(vertices=
                    [(0, 0),(1, 0),(1, -1),(0, -1)]))

fd_tile.shape.vertices = vertices

for i in range(1000000):
    t = world.CreateStaticBody(fixtures=fd_tile)
    world.DestroyBody(t)

    if i % 10 == 0:
        print('Ram Used: %f' % memory_used())

Looks like it's time to submit an issue to pybox2d. :D

The slow leaking doesn't matter in my case either, though it'd be nice to have this issue resolved. Would you be able to add the second vl.delete() statement to make the PR complete for the gym codebase?

@jackyoung96
Copy link
Contributor Author

Looks like this is an issue with Box2D, I was able to isolate the memory leak down to creating the tiles in _create_track. Despite calling DestroyBody on all of the tiles in _destroy, Box2D does not properly destroy the tiles sometimes. This memory leak can be reproduced with this minimum code:

import Box2D
from Box2D.b2 import fixtureDef, polygonShape

import psutil
import os

def memory_used():
    return psutil.Process(os.getpid()).memory_info().rss * 1e-6  # To megabyte


world = Box2D.b2World((0,0))


road1_l = (0, 0)
road1_r = (0, 1)
road2_l = (1, 0)
road2_r = (1, 1)
vertices = [road1_l, road1_r, road2_r, road2_l]

fd_tile = fixtureDef(
                shape = polygonShape(vertices=
                    [(0, 0),(1, 0),(1, -1),(0, -1)]))

fd_tile.shape.vertices = vertices

for i in range(1000000):
    t = world.CreateStaticBody(fixtures=fd_tile)
    world.DestroyBody(t)

    if i % 10 == 0:
        print('Ram Used: %f' % memory_used())

Looks like it's time to submit an issue to pybox2d. :D

The slow leaking doesn't matter in my case either, though it'd be nice to have this issue resolved. Would you be able to add the second vl.delete() statement to make the PR complete for the gym codebase?

Oh! I think I misunderstand your mention. I'm so sorry about that. I add vl.delete() in render_indicators function and make PR now. Thank you for your help.

@hirekk
Copy link

hirekk commented Jan 17, 2021

vl.delete() should also be added at the end of the render_road method (line 530), since the memory leak still occurs. As far as I can tell, this fixes the issue completely.

@yoavalon
Copy link

This solution works well for me. Memory remains low,

@SlipknotTN
Copy link

Me too

@xeviknal
Copy link

Any updates in this one? Do you think this PR is any close to be merged?

Copy link

@xeviknal xeviknal left a comment

Choose a reason for hiding this comment

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

I've been able to reproduce the issue. Once the PR is applied, the memory leak is gone or at least not exploding.

@jackyoung96
Copy link
Contributor Author

I've been able to reproduce the issue. Once the PR is applied, the memory leak is gone or at least not exploding.

Thanks for reproducing instead of me!!

@xeviknal
Copy link

xeviknal commented Feb 14, 2021

@pzhokhov @zuoxingdong I see you've been contributing quite a lot last months. Do you know what we can do to move this forward? are you maintainers? I am guessing this issue is making people to start forking. I'd be pretty good for this community to approach this nasty bug. thank you

A release with the fixing would be appreciated too 👼

@pzhokhov pzhokhov merged commit c8a6593 into openai:master Feb 16, 2021
zlig pushed a commit to zlig/gym that referenced this pull request Sep 6, 2021
…sue ) (openai#2096)

* car_racing.py memory leaking issue openai#2062 solving

* additional memory leaking resolve
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.

7 participants