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

update to ASM 229 #13451

Merged
merged 7 commits into from
Nov 2, 2022
Merged

update to ASM 229 #13451

merged 7 commits into from
Nov 2, 2022

Conversation

mjkkirschner
Copy link
Member

@mjkkirschner mjkkirschner commented Nov 1, 2022

Purpose

related:
https://git.autodesk.com/Dynamo/DynamoSelfServe/pull/83/

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

Release Notes

Support latest ADSK geometry kernel.

Reviewers

(FILL ME IN) Reviewer 1 (If possible, assign the Reviewer for the PR)

(FILL ME IN, optional) Any additional notes to reviewers or testers.

FYIs

(FILL ME IN, Optional) Names of anyone else you wish to be notified of

@@ -87,6 +87,7 @@ public void TestTextFromDynamoText()
#region Mesh Toolkit Tests

[Test]
[Category("Failure")]//TODO_update meshtoolkit for asm229
Copy link
Member Author

Choose a reason for hiding this comment

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

@aparajit-pratap you were right, I have to mark these tests as failing for now, I can look into updating meshtoolkit to latest AMT after this gets merged and then unmark these.

@mjkkirschner
Copy link
Member Author

mjkkirschner commented Nov 2, 2022

@aparajit-pratap @pinzart90 @sm6srw - this has now passed:
https://master-5.jenkins.autodesk.com/job/Dynamo/job/DynamoSelfServe/job/pullRequestValidation/7550/

though test time has increased to 45~ minutes from 30~ I am going to merge this.

My guesses are:

  1. fluke with the test machine (provisionally confirmed)
    2. By not copying ADP binaries we incur ASM startup costs when it searches the machine for ADP or fails to find it / throws.
    3. a mystery? Maybe new binaries are just slower?
    4. ASM team measured startup cost for loading AMT to not make any difference - but perhaps it does with our preloader?

@mjkkirschner mjkkirschner merged commit a364c9f into DynamoDS:master Nov 2, 2022
@mjkkirschner mjkkirschner deleted the asm229 branch November 2, 2022 14:15
@@ -92,6 +92,50 @@ private static List<string> ProductsWithASM
"ADPSDKUI.DLL",
"ADPSDKCORE.DLL"
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we want to continue preloading ADP dll's for ASM228?

Copy link
Member Author

Choose a reason for hiding this comment

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

hmm, good question - I will file a followup to look into this as we'd have to make changes on the LibG side as well.

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.

3 participants