2

I have application with 1 DataGridView, 1 DataTable etc.

My "execute" method (edited):

private void btnRunSQL_click()
{
        string strConnStr = tbConnStr.Text; // connection string from textbox
        string strSQL = tbSql.Text;         // query from textbox

        SqlDataAdapter dataAdapter = new SqlDataAdapter(strSQL, strConnStr);
        SqlCommandBuilder commandBuilder = new SqlCommandBuilder(dataAdapter);

        // clean memory
        // dtData DataTable is declared in main form class
        dtData = new DataTable(); 
        dataAdapter.Fill(dtData);

        showMemoryUsage();

 }

This is how im checking memory:

 public void showMemoryUsage()
 {
        Process proc = Process.GetCurrentProcess();
        this.Text = "Peak memory: " + proc.PeakWorkingSet64 / 1024 / 1024 + "MB";
        Application.DoEvents(); // force form refresh
 }

When I run this function multiple times - it uses more and more memory. Im working on very big data set (1000000 rows) and after few big queries I have to restart my app.

After running 1M rows query I have memory use about 900MB, second run 1100MB, 1300MB etc.

I thought re-initalizing DataTable will free my memory, but it does not. So I re-initialized BindingSource (connected with DataGridView), but it not helped too. Finally i commented my BindingSource and DataGridView.

Added later:

Disposing DataAdapter not helped.

I removed DataGridView and binding source. Not helped.

SOLUTION (I merged few answers and created test app without leaks)

using System;
using System.Data;
using System.Windows.Forms;
using System.Data.SqlClient;
using System.Diagnostics;

// to run this code you need form and controls:

// TextBox tbConnStr - textbox with SQL Server connection string
// TextBox tbSQL - for SQL query to run/test
// TextBox tbLog - for log display
// Button btnRunSQL with OnClick event set to proper method
// Button btnRunTest with OnClick event set to proper method

namespace Test_datatable
{
    public partial class Form1 : Form
    {

        DataTable dt; // i need this global

        public Form1()
        {
            InitializeComponent();
        }

        private void btnRunSQL_Click(object sender, EventArgs e)
        {
            log("Method starts.");

            string strConnStr = tbConnStr.Text;
            string strSQL = tbSQL.Text;

            using (SqlDataAdapter da = new SqlDataAdapter(strSQL, strConnStr))
            {
                using (SqlCommandBuilder cb = new SqlCommandBuilder(da))
                {

                    if (dt != null)
                    {
                        dt.Clear();
                        dt.Dispose();
                        log("DataTable cleared and disposed.");
                    }

                    dt = new DataTable();
                    da.Fill(dt);
                    log("DataTable filled.");

                }
            }

            log("Method ends.");
            tbLog.Text += Environment.NewLine;

        }

        // prints time, string and memory usage on textbox
        private void log(string text)
        {
            tbLog.Text += DateTime.Now.ToString() 
                + " "  + text + memory() + 
                Environment.NewLine;

            Application.DoEvents(); // force form refresh
        }

        // returns memory use as string, example: "Memory: 123MB"
        private string memory()
        {
            Process proc = Process.GetCurrentProcess();
            return " Peak memory: " + (proc.PeakWorkingSet64 / 1024 / 1024).ToString() + "MB ";

        }

        // test method for 10 runs
        private void btnRunTest_Click(object sender, EventArgs e)
        {
            for (int i = 0; i < 10; i++)
            {
                btnRunSQL_Click(new object(), new EventArgs());
            }
        }
    }
}
Kamil
  • 13,363
  • 24
  • 88
  • 183
  • 3
    If this is your real code, then the BindingSource is never used and I would not expect a memory-leak. Anything else happening? – H H Apr 26 '13 at 13:54
  • This is simplified code, BindingSource is used to show data on DataGridView. – Kamil Apr 26 '13 at 13:55
  • 1
    I think the simplification is hiding the true problem. – H H Apr 26 '13 at 13:58
  • @Kamil, when tracking down memory consumption issues, you have to look at the program as a whole. Consider this, change your code from `dtData = new DataTable();` to `DataTable dtData = new DataTable();` and I would expect no change (i.e. I don't think this is where the memory consumption is coming from). You need to start commenting out code, and bringing stuff in one piece at a time, until you find consumption issues -then post that code (the isolated code -not guessed code) here. – Mike Perrenoud Apr 26 '13 at 13:58
  • SqlDataAdapter and SqlCommandBuilder are disposable. not sure if using(var dataAdapter = new SqlDataAdapter(strSQL, strConnStr)){...} makes sense here – Joel Apr 26 '13 at 13:59
  • 2
    Are you restarting the app because you *think* memory usage is high or are you getting an Out of Memory Exception? – Mesh Apr 26 '13 at 14:05
  • @Adrian i updated my code, this is how im checking memory. And yes, finally im getting "out of memory" exception. – Kamil Apr 26 '13 at 14:25
  • Have you tried disposing of **both** `dataAdapter` and `commandBuilder` as some have suggested? Also, where else are you using `dtData`? What happens if you comment out all other places that reference it so that the only place it is used is this function? Do you still get a memory leak when you click the button then? – Jon Senchyna Apr 26 '13 at 14:35
  • @Kamil can you insert showMemoryUsage() before creating new table, and before filling it? It should show you where the leak occures. Besides try to change dtData from global to local. I suppose it could be the leak of GC does not rise despose for your datateble because of some object references it. – Alex Apr 26 '13 at 14:47
  • @voo The GC will not call `.Dispose()` for you; it must explicitly be called. (Yes, you could implement `Finalize` and have it call `Dispose` to make the GC do the work for you, but it is not done for you out of the box and that's still "explicitly" calling it.) – Jon Senchyna Apr 26 '13 at 15:00
  • @voo Thanks for suggestion. I try this, however it will be hard, because i need that DataTable in many other methods (so when its local i have to pass it via parameter everywhere). I will create separate application for testing this. – Kamil Apr 26 '13 at 15:01

4 Answers4

3

Best guess: there remain links from the GUI to the old DataTables through the Bindings. But the actual code is missing.

My preferred approach in this context:

if (bs != null)     bs.Clear();      // most likely solution
if (dtData != null) dtData.Clear();  // won't hurt

dtData = new DataTable(); 
bs = new BindingSource(); 
H H
  • 263,252
  • 30
  • 330
  • 514
  • I totally get rid of binding source and datagridview. Looks like clearing data before re-initializing helped a little, but not completely. – Kamil Apr 26 '13 at 14:14
  • What you probably need is `dataGridView1.DataSource = null;` at the right moment. But you didn't post enough code. – H H Apr 26 '13 at 15:19
  • Final conclusion: I created test application (see updated question) with clearing and disposing objects. DataTable leak problem is fixed for sure, but I need to fix code related with DataGridView, because I have some leaks there too. – Kamil Apr 26 '13 at 19:34
2

For starters, you should be disposing:

private void btnRunSQL_click()
{
        string strConnStr = tbConnStr.Text; // connection string from textbox
        string strSQL = tbSql.Text;         // query from textbox

        using(SqlDataAdapter dataAdapter = new SqlDataAdapter(strSQL, strConnStr))
        {
           using(SqlCommandBuilder commandBuilder = new SqlCommandBuilder(dataAdapter))
           {
               // clean memory
               // dtData DataTable and BindingSource bs are declared in main form class
               dtData = new DataTable(); 
               bs = new BindingSource(); 

               dataAdapter.Fill(dtData);
          }
      }
 }

But I believe at least part of your problem is included in the code you have not shown us.

EDIT: Please show us how you are using the BindingSource.

EDIT2: You are using PeakWorkignSet64, is your application running as 64 bit? If not this property will not be accurate. Also, try System.GC.GetTotalMemory(true) and tell us what that reports.

EkoostikMartin
  • 6,831
  • 2
  • 33
  • 62
0

SqlDataAdapter and SqlCommandBuilder are disposable, you could try something like this

private void btnRunSQL_click()
{
        string strConnStr = tbConnStr.Text; // connection string from textbox
        string strSQL = tbSql.Text;         // query from textbox

        using(var dataAdapter = new SqlDataAdapter(strSQL, strConnStr))
        using(var commandBuilder = new SqlCommandBuilder(dataAdapter)) {
             dtData = new DataTable(); 
             bs = new BindingSource(); 

             dataAdapter.Fill(dtData);
        }
}

I think this won't fix your memory problem but since SqlDataAdapter and SqlCommandBuilder are disposable it is a good idea todo so.

Joel
  • 4,862
  • 7
  • 46
  • 71
-1

Why don't you just re-fill your old dtData?

By creating a new instance of dtData, the old one will be flushed away by the Garbage Collector some time. You could also call the GC manually but I don't recommend that if there is a better solution.

Edit: To make this more clear, try using this function:

private void btnRunSQL_click()
{
    string strConnStr = tbConnStr.Text; // connection string from textbox
    string strSQL = tbSql.Text;         // query from textbox

    using(SqlDataAdapter dataAdapter = new SqlDataAdapter(strSQL, strConnStr))
    {
        using(SqlCommandBuilder commandBuilder = new SqlCommandBuilder(dataAdapter))
        {
            // edited after comment of  Jon Senchyna
            dtData.Clear();
            dataAdapter.Fill(dtData);
        }
     }
}
Jon Senchyna
  • 7,867
  • 2
  • 26
  • 46
WhileTrueSleep
  • 1,524
  • 1
  • 19
  • 32
  • 2
    I do... Like this: `dtData = new DataTable();` – Kamil Apr 26 '13 at 13:56
  • 1
    Isn't that what he is doing? – Joetjah Apr 26 '13 at 13:56
  • no u create a new instance of it... u dont delete the old or override it! – WhileTrueSleep Apr 26 '13 at 13:58
  • in that case, this might add in the discussion: http://stackoverflow.com/questions/913228/should-i-dispose-dataset-and-datatable – Joetjah Apr 26 '13 at 13:59
  • 1
    Just calling `dataAdapter.Fill(dtData)` does not handle deleted rows. It will add new rows and update changed rows **if there is a primary key on the table**. If there is not, it will simply append everything to the table (creating duplicate rows). – Jon Senchyna Apr 26 '13 at 14:07
  • @JonSenchyna newly re-initialized `DataTable` is empty. There are no row duplicates (i was checking row count on DataTable). – Kamil Apr 26 '13 at 14:22
  • 1
    @Kamil There was no "newly re-initialized `DataTable`". The answer was suggesting removing the re-initialization and just using `DataAdapter.Fill()` on the existing `dtData`. There is now a call to `DataTable.Clear()`, which should both handle deletions and prevent duplicate rows. – Jon Senchyna Apr 26 '13 at 14:26