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

factor out IO type constants #105

Closed
pixelzoom opened this issue Nov 9, 2018 · 2 comments
Closed

factor out IO type constants #105

pixelzoom opened this issue Nov 9, 2018 · 2 comments
Assignees

Comments

@pixelzoom
Copy link
Contributor

pixelzoom commented Nov 9, 2018

In light of how IO types increase memory footprint (see phetsims/tandem#71), consider factoring out these sim-specific phetioTypes values:

    // related to Quadratic
    PropertyIO( NullableIO( QuadraticIO ) )
    DerivedPropertyIO( QuadraticIO )
    DerivedPropertyIO( NullableIO( QuadraticIO ) ) 

    // related to Vector2
    PropertyIO( Vector2IO )
    DerivedPropertyIO( Vector2IO )
    DerivedPropertyIO( NullableIO( Vector2IO ) )
@pixelzoom pixelzoom self-assigned this Nov 9, 2018
@pixelzoom
Copy link
Contributor Author

pixelzoom commented Nov 9, 2018

If this turns out to be a significant memory savings, the same should be done in sim-specific code for other instrumented sims.

@pixelzoom
Copy link
Contributor Author

pixelzoom commented Nov 10, 2018

Mentioning @samreid, since we had discussed the potential benefit of this on Slack, in relation to phetsims/tandem#71.

I tried factoring out constants for the phetioType values identified in #105 (comment). Here are the heap snapshots on Chrome 70.0.3538.77 + macOS 10.11.6, ?ea&brand=phet&fuzz, before and after factoring out these constants.

Time (minutes) Before After
0 44.2 MB 44.0 MB
2 51.5 MB 51.9 MB

Since there is no obvious memory reduction, I'm not going to commit the changes. I think it's clearer in this case to have IO type instances next to the phetioType keys.

For posterity, here's what I added to GQConstants, but did not commit:

    // phetioType values related to Quadratic, see https://github.com/phetsims/graphing-quadratics/issues/105
    PROPERTY_IO_NULLABLE_IO_QUADRATIC_IO: PropertyIO( NullableIO( QuadraticIO ) ),
    DERIVED_PROPERTY_IO_QUADRATIC_IO: DerivedPropertyIO( QuadraticIO ),
    DERIVED_PROPERTY_IO_NULLABLE_IO_QUADRATIC_IO: DerivedPropertyIO( NullableIO( QuadraticIO ) ),

    // phetioType values related to Vector2, see https://github.com/phetsims/graphing-quadratics/issues/105
    PROPERTY_IO_VECTOR2_IO: PropertyIO( Vector2IO ),
    DERIVED_PROPERTY_IO_VECTOR2_IO: DerivedPropertyIO( Vector2IO ),
    DERIVED_PROPERTY_IO_NULLABLE_IO_VECTOR2_IO: DerivedPropertyIO( NullableIO( Vector2IO ) )

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

1 participant