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

Implement SDL-0187 Restructure iOS Threading / Queueing #1265

Closed

Conversation

joeljfischer
Copy link
Contributor

@joeljfischer joeljfischer commented May 16, 2019

Fixes #1028

This PR is ready for review.

Risk

This PR makes no API changes.

Testing Plan

Unit and smoke tests

Summary

This PR restructures the threading in the library to use an "execution context" structure.

Changelog

Enhancements
  • Restructure the threading in the library to use an "execution context" structure.

Tasks Remaining:

  • Test removing the serialTransport queue and use the processing queue
  • Test concurrent vs. serial in concurrentProcessing queue

CLA

@joeljfischer joeljfischer added this to the 6.3.0 milestone May 16, 2019
@joeljfischer joeljfischer self-assigned this May 16, 2019
@joeljfischer joeljfischer added this to In progress in v6.3 via automation May 16, 2019
@joeljfischer joeljfischer changed the title WIP: Restructure iOS Threading / Queueing Restructure iOS Threading / Queueing Jun 5, 2019
@joeljfischer joeljfischer added the proposal Accepted SDL Evolution Proposal label Jun 7, 2019
@joeljfischer joeljfischer changed the title Restructure iOS Threading / Queueing WIP: Restructure iOS Threading / Queueing Jun 7, 2019
@joeljfischer joeljfischer changed the title WIP: Restructure iOS Threading / Queueing Restructure iOS Threading / Queueing Jun 10, 2019
Copy link
Contributor

@NicoleYarroch NicoleYarroch left a comment

Choose a reason for hiding this comment

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

Left some comments

Example Apps/Example ObjC/ProxyManager.m Outdated Show resolved Hide resolved
@@ -60,6 +60,11 @@
</BuildableReference>
</MacroExpansion>
<AdditionalOptions>
<AdditionalOption
key = "NSZombieEnabled"
value = "YES"
Copy link
Contributor

Choose a reason for hiding this comment

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

This is for debugging, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, but since it's on the test scheme, it can just stay enabled.

SmartDeviceLink/SDLLifecycleManager.m Outdated Show resolved Hide resolved
@@ -92,7 +92,7 @@ - (void)sdl_sendFile:(SDLFile *)file mtuSize:(NSUInteger)mtuSize withCompletion:

// Wait for all packets be sent before returning whether or not the upload was a success
__weak typeof(self) weakself = self;
dispatch_group_notify(putFileGroup, dispatch_get_main_queue(), ^{
dispatch_group_notify(putFileGroup, [SDLGlobals sharedGlobals].sdlProcessingQueue, ^{
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't this a breaking change, since devs will expect to get the response on the main thread?

v6.3 automation moved this from In progress to Review in progress Jun 10, 2019
@theresalech theresalech removed this from Review in progress in v6.3 Jun 12, 2019
@theresalech theresalech modified the milestones: 6.3.0, Future Jun 12, 2019
* See todo for current location
@theresalech theresalech added this to In progress in v6.4 via automation Jun 13, 2019
@joeljfischer joeljfischer changed the title Restructure iOS Threading / Queueing Implement SDL-0187 Restructure iOS Threading / Queueing Jul 15, 2019
@joeljfischer
Copy link
Contributor Author

Closed due to wrong branch name. New version is at #1348.

v6.4 automation moved this from In progress to Done Jul 15, 2019
@joeljfischer joeljfischer deleted the feature/issue_0187_restructure_ios_threading branch February 7, 2020 16:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
proposal Accepted SDL Evolution Proposal
Projects
No open projects
v6.4
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants