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

Another Memory Leak in Table.updateModel #51

Closed
816-8055 opened this issue Nov 24, 2016 · 2 comments
Closed

Another Memory Leak in Table.updateModel #51

816-8055 opened this issue Nov 24, 2016 · 2 comments

Comments

@816-8055
Copy link
Contributor

Dear Mr. Farrell,

I very much appreciate the time and effort you put into this project!
I'm glad that I found your project because otherwise I'd have to write my own table-widget for pandas.DataFrame.

After #47 was fixed, I found out that the consumed memory still grows when table data is updated via updateModel.
While looking at the code, I realized that parts of the UI are newly generated on each call of Table.show (which is called by updateModel) without "destroying" all old widgets properly.
To fix this, some bigger changes are required - I think.

This quick fix unluckily produces an error in some cases:

--- a/pandastable/core.py
+++ b/pandastable/core.py
@@ -2868,11 +2868,9 @@ class Table(Canvas):
         self.cols = self.model.getColumnCount()
         self.tablewidth = (self.cellwidth)*self.cols
         if hasattr(self, 'tablecolheader'):
-            self.tablecolheader.destroy()
-            self.rowheader.destroy()
             self.selectNone()
-        self.show()
-        return
+        else:
+            self.show()

     def new(self):
         """Clears all the data and makes a new table"""

Running my test program from below gives me following values for the current core.py:

38.30 cycles/second
23.59 kB/cycle
18.15 MB gained

After applying the patch from above:

58.35 cycles/second
0.52 kB/cycle
0.63 MB gained

Obviously the patch has also an effect on the performance. #31

test program:

import psutil
import tkinter as tk
import pandas
import pandastable

DURATION = 20  # seconds the program should run


def update_model():
    df = pandas.DataFrame(list(range(N, N+10)))
    table.updateModel(pandastable.TableModel(df))


def update_title(set_smu=False):
    global N, S
    if set_smu:
        global smu
        smu = me.memory_full_info().uss
    mem = me.memory_full_info().uss
    update_model()
    table.redraw()
    diff = me.memory_full_info().uss - mem
    S += diff
    N += 1
    root.title('total: {:.2f}MB - added per redraw: {:.2f}kB'
               .format(mem / 1000**2, (S/N)/1000))
    root.after(10, update_title)

if __name__ == '__main__':
    N = 0
    S = 0
    me = psutil.Process()
    root = tk.Tk()
    frame = tk.Frame(root)
    table = pandastable.Table(frame)
    table.show()
    frame.pack(fill='both', expand=True)
    root.after(1000, update_title, True)
    update_model()
    root.update()
    root.after(1000 + DURATION*1000, root.destroy)
    root.mainloop()
    print('{:.2f} cycles/second'.format(N/DURATION))
    print('{:.2f} kB/cycle'.format((S/N)/1000))
    print('{:.2f} MB gained'.format((me.memory_full_info().uss-smu)/1000**2))
@dmnfarrell
Copy link
Owner

Thanks, it's good to have someone who can read the code and make meaningful changes. self.show should only be called when the table is reloaded anew or when created so would this fix have a significant effect? either way I am happy to make the changes if you can address the errors your quick fix introduces. I guess the headers should be destroyed in the show method itself.
There is a performance issue in rendering tables when the window frame is larger (more rows/cols visible), hence the issue you referenced. It would be great to fix that too.

@816-8055
Copy link
Contributor Author

816-8055 commented Mar 13, 2017

You 're right, it's only an issue when the widget is updated often. In my case the widget is used to show the data under the cursor of a matplotlib figure and is thus updated very often.

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

No branches or pull requests

2 participants