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

3MB/sec of allocations during dragging? #74

Closed
samreid opened this issue Jul 7, 2017 · 14 comments
Closed

3MB/sec of allocations during dragging? #74

samreid opened this issue Jul 7, 2017 · 14 comments
Assignees

Comments

@samreid
Copy link
Member

samreid commented Jul 7, 2017

image

May be causing #62

@samreid samreid self-assigned this Jul 7, 2017
samreid added a commit to phetsims/circuit-construction-kit-common that referenced this issue Jul 7, 2017
samreid added a commit to phetsims/circuit-construction-kit-common that referenced this issue Jul 7, 2017
samreid added a commit to phetsims/circuit-construction-kit-common that referenced this issue Jul 7, 2017
@samreid
Copy link
Member Author

samreid commented Jul 7, 2017

After the above commits, Chrome is reporting 2.95MB/sec. I ran another "before commit" run and saw 2.83MB/sec. Is everything in the noise here?

@samreid
Copy link
Member Author

samreid commented Jul 7, 2017

On the other hand, after these changes I was able to use the app for 5+ minutes on the iPad2, including dragging out every component and making circuits without any crashes. Perhaps we should do some more of this.

@samreid
Copy link
Member Author

samreid commented Jul 8, 2017

I noticed that for 60 frames, CircuitLayerNode.dragVertex is called 248 times. Are we processing multiple events per animation frame? If so, how many? This seems like it could impact performance.

@samreid
Copy link
Member Author

samreid commented Jul 8, 2017

I found that FixedLengthCircuitElement.drag is called 2x per animation frame. Is this normal or some bug in CCK code?

@samreid
Copy link
Member Author

samreid commented Jul 8, 2017

Dragging wires still allocates ChargeNodes because the vertices move independently--we will need to optimize this or pool the ChargeNodes.

samreid added a commit to phetsims/circuit-construction-kit-common that referenced this issue Jul 8, 2017
@samreid
Copy link
Member Author

samreid commented Jul 8, 2017

Huge improvement for dragging wires in the above commit. Charges are still created (unpooled) when wires change length.

@samreid
Copy link
Member Author

samreid commented Jul 8, 2017

Reset all seems broken by the above changes.

@samreid
Copy link
Member Author

samreid commented Jul 15, 2017

I did a fresh recording of memory allocations when dragging a single battery, and it is up at 10.6MB/sec. I'm wondering if there is some inherent noise in these readings?

@samreid
Copy link
Member Author

samreid commented Jul 15, 2017

Likewise, Chrome is reporting around 2MB/sec garbage generated while nothing is happening.

UPADTE: only 0.4MB/sec garbage without ?ea

@samreid
Copy link
Member Author

samreid commented Jul 17, 2017

I hacked inherit like this to try to catch all invocations by PhET code:

// Copyright 2013-2015, University of Colorado Boulder

/**
 * Utility function for setting up prototypal inheritance.
 * Maintains supertype.prototype.constructor while properly copying ES5 getters and setters.
 * Supports adding functions to both the prototype itself and the constructor function.
 *
 * Usage:
 *
 * // Call the supertype constructor somewhere in the subtype's constructor.
 * function A() { scenery.Node.call( this ); };
 *
 * // Add prototype functions and/or 'static' functions
 * return inherit( scenery.Node, A, {
 *   customBehavior: function() { ... },
 *   isAnA: true
 * }, {
 *   someStaticFunction: function() { ...}
 * } );
 *
 * // client calls
 * new A().isAnA; // true
 * new scenery.Node().isAnA; // undefined
 * new A().constructor.name; // 'A'
 * A.someStaticFunction();
 *
 * @author Jonathan Olson <[email protected]>
 */
define( function( require ) {
  'use strict';

  var phetCore = require( 'PHET_CORE/phetCore' );
  var extend = require( 'PHET_CORE/extend' );

  /**
   * @param supertype           Constructor for the supertype.
   * @param subtype             Constructor for the subtype. Generally should contain supertype.call( this, ... )
   * @param prototypeProperties [optional] object containing properties that will be set on the prototype.
   * @param staticProperties [optional] object containing properties that will be set on the constructor function itself
   */
  function inherit( supertype, subtype, prototypeProperties, staticProperties ) {
    assert && assert( typeof supertype === 'function' );

    function F() {}

    F.prototype = supertype.prototype; // so new F().__proto__ === supertype.prototype

    subtype.prototype = extend( // extend will combine the properties and constructor into the new F copy
      new F(),                  // so new F().__proto__ === supertype.prototype, and the prototype chain is set up nicely
      { constructor: subtype }, // overrides the constructor properly
      prototypeProperties       // [optional] additional properties for the prototype, as an object.
    );

    //Copy the static properties onto the subtype constructor so they can be accessed 'statically'
    extend( subtype, staticProperties );

    var Constructor = function() {
      console.log('constructor');
      subtype.apply( this, arguments );
    };
    Constructor.prototype = extend( // extend will combine the properties and constructor into the new F copy
      new F(),                  // so new F().__proto__ === supertype.prototype, and the prototype chain is set up nicely
      { constructor: subtype }, // overrides the constructor properly
      prototypeProperties       // [optional] additional properties for the prototype, as an object.
    );

    //Copy the static properties onto the subtype constructor so they can be accessed 'statically'
    extend( Constructor, staticProperties );
    return Constructor; // pass back the subtype so it can be returned immediately as a module export
  }

  phetCore.register( 'inherit', inherit );

  return inherit;
} );

But I can't tell if it is working properly.

@samreid
Copy link
Member Author

samreid commented Jul 19, 2017

This may be fine now? Perhaps OK for QA testing.

@samreid
Copy link
Member Author

samreid commented Jul 22, 2017

The fate of this issue is tied with #62, I'll unassign to take it off my todo list for now.

@samreid samreid removed their assignment Jul 22, 2017
@samreid
Copy link
Member Author

samreid commented Oct 3, 2017

Now that #62 is clear, the only other reason this issue could be a problem is if there are performance problems using the sim. @phet-steele has iPad2 performance during dragging generally been acceptable?

@phet-steele
Copy link
Contributor

@samreid I don't really know how to quantify it, but maybe my results show 2 MB/s?
screen shot 2017-10-04 at 10 00 42 am

has iPad2 performance during dragging generally been acceptable?

Do things trail behind my finger? Yes.
When there are many elements, are drags smooth? No.
Is it annoying to make circuits? Nope! Closing.

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

No branches or pull requests

2 participants