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

Fix memory leak when calling .destroy #745

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

mreishus
Copy link
Contributor

Creating a query builder, loading it with data, then destroying it was
causing some memory to be retained in the browser.

This should fix.

This fix was created by my colleague w/ @claudior2184 .

Merge request checklist

  • I read the guidelines for contributing
  • I created my branch from dev and I am issuing the PR to dev
  • I didn't pushed the dist directory
  • Unit tests are OK
  • If it's a new feature, I added the necessary unit tests
  • If it's a new language, I filled the __locale and __author fields

Creating a query builder, loading it with data, then destroying it was
causing some memory to be retained in the browser.

This should fix.

This fix was created by my colleague w/ @claudior2184 .
@mistic100
Copy link
Owner

I am not against this change but are you sure it fix anything ? I don't see any reason for these properties not be GC.

@mreishus
Copy link
Contributor Author

Use this test case, save as HTML and drag to browser.

<!-- Imports -->
<link rel="stylesheet" href="https://maxcdn.bootstrapcdn.com/bootstrap/3.3.6/css/bootstrap.min.css" integrity="sha384-1q8mTJOASx8j1Au+a5WDVnPi2lkFfwwEAa8hDDdjZlpLegxhjVME1fgjWPGmkzs7" crossorigin="anonymous">

<script src="https://code.jquery.com/jquery-2.2.3.min.js"></script>

<script src="https://maxcdn.bootstrapcdn.com/bootstrap/3.3.6/js/bootstrap.min.js" integrity="sha384-0mSbJDEHialfmuBBQP6A4Qrprq5OVfW37PRR3j5ELqxss1yVqOtnepnHVP9aJ7xS" crossorigin="anonymous"></script>

<script src="http://cdn.jsdelivr.net/npm/jQuery-QueryBuilder/dist/js/query-builder.standalone.min.js">
</script>

<!-- Application -->
<div>
  <button id="1kbtn">Make 1k QB</button>
  
  <div id="builder-basic" />
  
</div>

<script>
function createQB() {
  var rules_basic = {
    condition: 'AND',
    rules: [{
      id: 'price',
      operator: 'less',
      value: 10.25
    }, {
      condition: 'OR',
      rules: [{
        id: 'category',
        operator: 'equal',
        value: 2
      }, {
        id: 'category',
        operator: 'equal',
        value: 1
      }]
    }]
  };
  var filters_basic = [
  {
      id: 'name',
      label: 'Name',
      type: 'string'
    }, {
      id: 'category',
      label: 'Category',
      type: 'integer',
      input: 'select',
      values: {
        1: 'Books',
        2: 'Movies',
        3: 'Music',
        4: 'Tools',
        5: 'Goodies',
        6: 'Clothes'
      },
      operators: ['equal', 'not_equal', 'in', 'not_in', 'is_null', 'is_not_null']
    }, {
      id: 'in_stock',
      label: 'In stock',
      type: 'integer',
      input: 'radio',
      values: {
        1: 'Yes',
        0: 'No'
      },
      operators: ['equal']
    }, {
      id: 'price',
      label: 'Price',
      type: 'double',
      validation: {
        min: 0,
        step: 0.01
      }
    }, {
      id: 'id',
      label: 'Identifier',
      type: 'string',
      placeholder: '____-____-____',
      operators: ['equal', 'not_equal'],
      validation: {
        format: /^.{4}-.{4}-.{4}$/
      }
    }
  ];

  $('#builder-basic').queryBuilder({
    plugins: ['bt-tooltip-errors'],
    filters: filters_basic,
    rules: rules_basic
  });

  $('#btn-reset').on('click', function() {
    $('#builder-basic').queryBuilder('reset');
  });

  $('#btn-set').on('click', function() {
    $('#builder-basic').queryBuilder('setRules', rules_basic);
  });

  $('#btn-get').on('click', function() {
    var result = $('#builder-basic').queryBuilder('getRules');

    if (!$.isEmptyObject(result)) {
      alert(JSON.stringify(result, null, 2));
    }
  });
  
}

function destroyQB() {
  $("#builder-basic").queryBuilder("destroy");
}

function make1kQB() {
  console.log('make1kqb begin');
  for (var i = 0; i <= 1000; i++) {
    createQB();
    //console.log(i);
    destroyQB();
  }
  console.log('make1kqb end');
}
// on document load
$(function(){
    $("#1kbtn").click(function(){
        make1kQB();
    }); 


});

</script>

How to check

chrome_2018-11-12_11-25-54

@mistic100
Copy link
Owner

mistic100 commented Nov 23, 2018

I made a rough test by overriding the method https://jsfiddle.net/zk9qt5fj/2/
It stills goes up, but more slowly (~5MB each run instead of ~10MB) there must some others references

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.

None yet

2 participants