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

Avoid creating TCOs while rendering #5436

Closed

Conversation

Reflexe
Copy link
Member

@Reflexe Reflexe commented Apr 4, 2020

No description provided.

@LmmsBot
Copy link

LmmsBot commented Apr 4, 2020

🤖 Hey, I'm @LmmsBot from github.com/lmms/bot and I made downloads for this pull request, click me to make them magically appear! 🎩

Windows

macOS

🤖
{"platform_name_to_artifacts": {"Windows": [{"artifact": {"title": {"title": "32-bit", "platform_name": "Windows"}, "link": {"link": "https://1474-143851518-gh.circle-artifacts.com/0/lmms-1.2.0-rc6.779-mingw-win32.exe"}}, "build_link": "https://circleci.com/gh/Reflexe/lmms/1474?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}, {"artifact": {"title": {"title": "64-bit", "platform_name": "Windows"}, "link": {"link": "https://1478-143851518-gh.circle-artifacts.com/0/lmms-1.2.0-rc6.779-mingw-win64.exe"}}, "build_link": "https://circleci.com/gh/Reflexe/lmms/1478?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}, {"artifact": {"title": {"title": "32-bit", "platform_name": "Windows"}, "link": {"link": "https://ci.appveyor.com/api/buildjobs/up3njfc1rbo9srwp/artifacts/build/lmms-1.2.1-msvc2017-win32.exe"}}, "build_link": "https://ci.appveyor.com/project/Lukas-W/lmms/builds/31949511"}, {"artifact": {"title": {"title": "64-bit", "platform_name": "Windows"}, "link": {"link": "https://ci.appveyor.com/api/buildjobs/oc1t9rf0gli88nv0/artifacts/build/lmms-1.2.1-msvc2017-win64.exe"}}, "build_link": "https://ci.appveyor.com/project/Lukas-W/lmms/builds/31949511"}], "macOS": [{"artifact": {"title": {"title": "", "platform_name": "macOS"}, "link": {"link": "https://1475-143851518-gh.circle-artifacts.com/0/lmms-1.2.0-rc6.779-mac10.13.dmg"}}, "build_link": "https://circleci.com/gh/Reflexe/lmms/1475?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}]}, "commit_sha": "dbe60d8537da739c39acf1e8303fbb93da82d09b"}

@Reflexe Reflexe force-pushed the avoid_creating_tco_while_rendering branch from 9d28cfd to 61ffb2b Compare April 4, 2020 10:08
Reflexe added 2 commits April 4, 2020 13:09
requestChangeInModel.

Deprecate direct calls to requestChangeInModel.
This commit renames createTCO to unsafeCreateTCO and
add a mixer guard on the new createTCO.
@Reflexe Reflexe force-pushed the avoid_creating_tco_while_rendering branch from 61ffb2b to dbe60d8 Compare April 4, 2020 10:10
@Reflexe Reflexe requested a review from PhysSong April 4, 2020 10:11
@PhysSong
Copy link
Member

PhysSong commented Apr 9, 2020

I think this is overkill. Creating a TCO shouldn't cause any race conditions with Mixer as you addressed in this PR, but I think you can somehow minimize actions within the requestChangeInModel()~doneChangeInModel() block. We should take care of realtime safety while considering thread safety as well.
The first commit looks good, though.

@PhysSong
Copy link
Member

@Reflexe Is it okay to cherry-pick the first commit to master and close this PR for now?

@Reflexe
Copy link
Member Author

Reflexe commented Jun 18, 2022 via email

@PhysSong
Copy link
Member

Done via 371f7f5, with minor changes.

@PhysSong PhysSong closed this Jun 18, 2022
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