Skip to content

update to ASM 229#13451

Merged
mjkkirschner merged 7 commits intoDynamoDS:masterfrom
mjkkirschner:asm229
Nov 2, 2022
Merged

update to ASM 229#13451
mjkkirschner merged 7 commits intoDynamoDS:masterfrom
mjkkirschner:asm229

Conversation

@mjkkirschner
Copy link
Member

@mjkkirschner mjkkirschner commented Nov 1, 2022 •
edited
Loading

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

mjkkirschner
mjkkirschner commented Nov 2, 2022
View reviewed changes
test/Libraries/WorkflowTests/PackageValidationTest.cs
#region Mesh Toolkit Tests

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

@mjkkirschner mjkkirschner Nov 2, 2022

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 •
edited
Loading

@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
aparajit-pratap
aparajit-pratap reviewed Nov 2, 2022
View reviewed changes
src/Tools/DynamoShapeManager/Utilities.cs
@@ -92,6 +92,50 @@ private static List<string> ProductsWithASM
"ADPSDKUI.DLL",
"ADPSDKCORE.DLL"
Copy link
Contributor

@aparajit-pratap aparajit-pratap Nov 2, 2022

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

@mjkkirschner mjkkirschner Nov 2, 2022

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

Reviewers

@aparajit-pratap aparajit-pratap aparajit-pratap left review comments

@pinzart90 pinzart90 pinzart90 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@mjkkirschner @aparajit-pratap @pinzart90