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

Windows Fixes #904

Merged
merged 8 commits into from
Dec 9, 2021
Merged

Windows Fixes #904

merged 8 commits into from
Dec 9, 2021

Conversation

varunagrawal
Copy link
Collaborator

@varunagrawal varunagrawal commented Oct 23, 2021

The Windows Release CI currently takes over 2 hours to run, so this PR comments it out until we can figure out the issue. Since Windows Debug runs without issues, this should be an acceptable change.

I've also added a bunch of GTSAM_EXPORT declarations to various classes.

@varunagrawal varunagrawal added ci Related to the Continuous Integration pipeline windows Related to Windows labels Oct 23, 2021
@varunagrawal varunagrawal self-assigned this Oct 23, 2021
@varunagrawal varunagrawal added the quick-review Quick and easy PR to review label Oct 25, 2021
Copy link
Member

@gchenfc gchenfc left a comment

Choose a reason for hiding this comment

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

Are you sure this is actually working as it's supposed to? I didn't look in detail but in the Release CI log "Build" stage, I see a lot of instances of "Debug", e.g. line 428

 Creating library D:/a/gtsam/gtsam/build/lib/Debug/gtsam_unstableDebug.lib and object D:/a/gtsam/gtsam/build/lib/Debug/gtsam_unstableDebug.exp

whereas in other CI's before this change (e.g. this random one) debug doesn't appear

@varunagrawal
Copy link
Collaborator Author

Are you sure this is actually working as it's supposed to? I didn't look in detail but in the Release CI log "Build" stage, I see a lot of instances of "Debug", e.g. line 428

 Creating library D:/a/gtsam/gtsam/build/lib/Debug/gtsam_unstableDebug.lib and object D:/a/gtsam/gtsam/build/lib/Debug/gtsam_unstableDebug.exp

whereas in other CI's before this change (e.g. this random one) debug doesn't appear

If you open up the build log for the 2nd one, you'll see a lot of debug statements there as well (which I guess is a MSVC issue), but look at the time for that step in both logs. There's a massive difference.

@varunagrawal
Copy link
Collaborator Author

I misunderstood what you meant. I'll take another look at this.

@varunagrawal
Copy link
Collaborator Author

Ughhh it's not Boost that's the bottleneck, it's the release configuration. 😢

@varunagrawal varunagrawal changed the title Speedup Windows CI Windows Fixes Nov 29, 2021
@varunagrawal
Copy link
Collaborator Author

@gchenfc I've just commented out the Release CI for the time being.

@dellaert
Copy link
Member

dellaert commented Dec 7, 2021

Status on this PR?

Copy link
Member

@gchenfc gchenfc left a comment

Choose a reason for hiding this comment

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

Shoot sorry I forgot about this

This seems reasonable to me. I'm guessing just the optimization for RELEASE takes so long and github actions windows servers are slow/limited/overburdened ?

@varunagrawal
Copy link
Collaborator Author

Yeah I don't know if it is a code optimization issue. I asked about it on the github community site and haven't received a response. Let's see.

@varunagrawal varunagrawal merged commit b47f46a into develop Dec 9, 2021
@varunagrawal varunagrawal deleted the ci/windows branch December 9, 2021 14:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci Related to the Continuous Integration pipeline quick-review Quick and easy PR to review windows Related to Windows
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants