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

DYN-4229 GraphicDataProvider optimization for repeated calls to get similar object types for tessellation #11920

Merged

Conversation

saintentropy
Copy link
Contributor

@saintentropy saintentropy commented Aug 7, 2021

Purpose

https://jira.autodesk.com/browse/DYN-4229

This optimization is to short-circuit the repeated search for and allocation of numerous CLRObjectMarshaller objects during UpdateRenderPackageAsyncTask execution. Right now, within UpdateRenderPackageAsyncTask, we call mirrorData.GetData() for every object we are attempting to tessellate. Within that call, a new instance of an ObjectMarshaller allocated based on the objects type information. This optimization caches the ObjectMarshaller and reuses it if the object type is the same for repeated calls.

Declarations

Check these if you believe they are true

  • The codebase is in a better state after this PR
  • Is documented according to the standards
  • The level of testing this PR includes is appropriate
  • User facing strings, if any, are extracted into *.resx files
  • All tests pass using the self-service CI.
  • Snapshot of UI changes, if any.
  • Changes to the API follow Semantic Versioning and are documented in the API Changes document.
  • This PR modifies some build requirements and the readme is updated

Reviewers

TBD

FYIs

@jasonstratton @QilongTang

@saintentropy saintentropy changed the title GraphicData provider ot GraphicDataProvider optimization for repeated calls to get similar object types for tessellation Aug 7, 2021
@saintentropy saintentropy removed the WIP label Aug 9, 2021
@saintentropy
Copy link
Contributor Author

In a test graph with a lot of repeated calls to Circle.ByPointRadius this call reduced the time spent in this method from .5s to .07s and also reduced temporary memory allocation by 90%.

@saintentropy saintentropy changed the title GraphicDataProvider optimization for repeated calls to get similar object types for tessellation DYN-4229 GraphicDataProvider optimization for repeated calls to get similar object types for tessellation Oct 18, 2021
Copy link
Contributor

@sm6srw sm6srw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great! This looks solid to me. I did spend some time making sure Clear gets called when it should be called and it looks good.

Comment on lines +145 to +146
private static Interpreter interpreter;
private static ProtoFFI.FFIObjectMarshaler marshaler;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do these need to be static?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They are static because you only need one Interpreter and FFIObjectMarshaller for the GraphicDataProvider class until the runtime is reset. If you made the instances you would allocate permanently an interpreter and marshaller for every mirror data object and that would be worse than temporary allocation

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see that the graphicdataprovider member for mirrordata is static too so it looks like there will only be one instance of graphicdataprovider ever but I guess it doesn't hurt to make these static here as well.

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

Successfully merging this pull request may close these issues.

3 participants