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

pngs do not clear object properly in some cases #203

Closed
doutriaux1 opened this Issue Jun 26, 2017 · 9 comments

Comments

Projects
None yet
3 participants
@doutriaux1
Member

doutriaux1 commented Jun 26, 2017

branch extra_obj contains the following and failing test

Note that

  • commenting out png line makes test work
  • removing the dims on png line (self.x.png('crap')) does make the test pass as well
import cdms2
import cdat_info
import time
import numpy
import basevcstest
import vcs

class VCSTestXtra(basevcstest.VCSBaseTest):
    def testNoXtra(self):
        s=self.clt("clt",time=(45,45,'cob'),longitude=(46.,46.,'cob'),squeeze=1)
        print s.shape
        self.x.portrait()

        N = 20

        elements = vcs.listelements()
        cpy = {}
        for e in elements:
            cpy[e] = vcs.elements[e].copy()

        mn = 10000
        times = []
        for i in range(N):
            start = time.time()
            self.x.plot(s)
            self.x.png("blah",width=1200,height=1600,units="pixels")
            self.x.clear()
            end = time.time()
            elapsed = end -start
            if mn>elapsed: mn = elapsed
            print i,elapsed,elapsed/mn
            times.append(elapsed)
            for e in elements:
                print "\t",e,len(vcs.elements[e]),len(cpy[e])
                if len(vcs.elements[e])!=len(cpy[e]):
                    print "\tMore elements in:",e,len(vcs.elements[e]),len(cpy[e]),len(vcs.elements[e])-len(cpy[e])
                self.assertEqual(len(vcs.elements[e]),len(cpy[e]))

@doutriaux1 doutriaux1 added the bug label Jun 26, 2017

@doutriaux1 doutriaux1 added this to the 2.10.2 milestone Jun 26, 2017

@danlipsa

This comment has been minimized.

Contributor

danlipsa commented Jun 27, 2017

@doutriaux1 @aashish24 Is this what causes the slowdown we heard about in the ACME meeting?

@doutriaux1

This comment has been minimized.

Member

doutriaux1 commented Jun 27, 2017

@danlipsa yes. I spent most yesterday on this. The cause is the configure/resize event.
When we resize (here to fit the png out size) we replot everything on the new size, but never delete the original objects (or some of them). This leads to lost objects. To make things worse after the png is plotted we resize again to the original size! So double leak! I'm not 100% on how I'm going to fix this.

@danlipsa

This comment has been minimized.

Contributor

danlipsa commented Jun 27, 2017

@doutriaux1 Great job narrowing this down. That resize is very convoluted indeed. I remember debugging that for vcdat. Should I look into this?

@doutriaux1

This comment has been minimized.

Member

doutriaux1 commented Jun 27, 2017

@danlipsa if you have time sure, but create your own branch please. Whoever gets to fix it first. I'm thinking of saving the user calls on the display. And do a full clear and replot on a new display. Any other idea?

@danlipsa

This comment has been minimized.

Contributor

danlipsa commented Jun 27, 2017

@doutriaux1 Not really. I think the root of the problem comes from the fact that resize calls plot, which saves stuff in vcs.elements. That slows things down as well. I look forward to when we'll move axes and labels to VTK. We won't need to do anything on resize at that point.
Even with the current implementation, I don't understand why we need a resize event.

Maybe I should let you fix this as I don't fully understand what is going on in there. It seems wasteful for both of us to work on the same thing. I can jump in if you want me too though.

@aashish24

This comment has been minimized.

Contributor

aashish24 commented Jun 27, 2017

@danlipsa yes. I spent most yesterday on this. The cause is the configure/resize event.
When we resize (here to fit the png out size) we replot everything on the new size, but never delete the original objects (or some of them). This leads to lost objects. To make things worse after the png is plotted we resize again to the original size! So double leak! I'm not 100% on how I'm going to fix this.

@doutriaux1 thanks for looking into it. I think we should fix the bug but I am wondering why we are doing resize in a batch script? Why not setup the size correctly in the beginning and in that way our users wouldnot hit the issue and give us some time to look into it.

@doutriaux1

This comment has been minimized.

Member

doutriaux1 commented Jun 27, 2017

@aashish24 it's from png, we can control the size of the output. Hence the batch script.

Regardless resizing on your screen in interact mode (or via GUI) will lead to the same leaks.

@doutriaux1

This comment has been minimized.

Member

doutriaux1 commented Jun 27, 2017

@danlipsa I think one of the reason we need a full replot is for cases where the user sets an aspect ratio. In this case we need to create a template whose datawc_x1/x2/y1/y2 match the correct aspect ration given the window size.

@danlipsa

This comment has been minimized.

Contributor

danlipsa commented Jun 27, 2017

@doutriaux1 I see.

doutriaux1 added a commit that referenced this issue Jun 28, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment