Skip to content

Load and save sessions asynchronously [DISCL-359]#47

Merged
rdumusc merged 1 commit intoBlueBrain:masterfrom
rdumusc:async
Jun 24, 2016
Merged

Load and save sessions asynchronously [DISCL-359]#47
rdumusc merged 1 commit intoBlueBrain:masterfrom
rdumusc:async

Conversation

@rdumusc
Copy link
Copy Markdown

@rdumusc rdumusc commented Jun 23, 2016

No description provided.

@dnachbaur
Copy link
Copy Markdown
Contributor

Nice feature!
To me it's a bit risky to include in the release, it's your call if you're confident. While the session is being loaded, the UI stays fully operational I guess, so a user can issue loading of several sessions or do other stuff in the UI which potentially conflicts with the background loading?

@rdumusc
Copy link
Copy Markdown
Author

rdumusc commented Jun 23, 2016

Yes, it is possible to issue successive calls if the loading takes a lot of time.
In theory a second call will block until the first one is finished, so it won't do much harm. But this has to be tested to make sure it works well in practice. Saving a session is safe and seem to work well, but the unit tests for loading cause a deadlock right now so I am trying to come up with a slightly safer solution.

@rdumusc rdumusc force-pushed the async branch 2 times, most recently from e21f51b to d8496e6 Compare June 24, 2016 08:15
@rdumusc
Copy link
Copy Markdown
Author

rdumusc commented Jun 24, 2016

Updated with a different approach to avoid any risk of deadlocking the main QEventLoop. The launcher also closes everything before sending the load command to provide immediate feedback before the loading happens in the background. This has to be tested on the real wall, but I am quite confident about this change. If this is not working well for users, we can always make the load blocking again by just waiting on the future.

@dnachbaur
Copy link
Copy Markdown
Contributor

+1

@rdumusc rdumusc merged commit 124a7e7 into BlueBrain:master Jun 24, 2016
@rdumusc rdumusc deleted the async branch June 24, 2016 12:50
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.

2 participants