Proposed to dispatch_sync the rename, rather than waiting on a rename queue group. #2

Merged
merged 1 commit into from Feb 11, 2012

Conversation

Projects
None yet
2 participants
@gmckenzi

Just a thought.
Wouldn't doing a dispatch_sync on the rename achieve the same behaviour with less code?
Warning: I haven't tried compiling/running this. Just made a quick edit directly in GitHub.

odrobnik added a commit that referenced this pull request Feb 11, 2012

Merge pull request #2 from gmckenzi/patch-1
Proposed to dispatch_sync the rename, rather than waiting on a rename queue group.

@odrobnik odrobnik merged commit 4b34946 into Cocoanetics:master Feb 11, 2012

@odrobnik

This comment has been minimized.

Show comment Hide comment
@odrobnik

odrobnik Feb 11, 2012

Collaborator

Builds fine, looks more simple. I'm not that sure of myself with GCD yet, so thanks for your help!

Collaborator

odrobnik commented Feb 11, 2012

Builds fine, looks more simple. I'm not that sure of myself with GCD yet, so thanks for your help!

@gmckenzi

This comment has been minimized.

Show comment Hide comment
@gmckenzi

gmckenzi Feb 11, 2012

No worries. You did all the heavy lifting. :-)

Note however that my proposed changes mean that a second caller to rename won't start until the first rename is finished. Whereas in your original code a second caller rename could queue-up before the first one finished -- it simply would wait for the previous rename to finish. In the end, the behaviour is effectively the same.

Gavin McKenzie

On 2012-02-11, at 10:56 AM, Oliver Drobnik wrote:

Builds fine, looks more simple. I'm not that sure of myself with GCD yet, so thanks for your help!


Reply to this email directly or view it on GitHub:
#2 (comment)

No worries. You did all the heavy lifting. :-)

Note however that my proposed changes mean that a second caller to rename won't start until the first rename is finished. Whereas in your original code a second caller rename could queue-up before the first one finished -- it simply would wait for the previous rename to finish. In the end, the behaviour is effectively the same.

Gavin McKenzie

On 2012-02-11, at 10:56 AM, Oliver Drobnik wrote:

Builds fine, looks more simple. I'm not that sure of myself with GCD yet, so thanks for your help!


Reply to this email directly or view it on GitHub:
#2 (comment)

@gmckenzi

This comment has been minimized.

Show comment Hide comment
@gmckenzi

gmckenzi Feb 11, 2012

Damn. I got my explanation wrong. Let me try again.

In the original implementation, additional callers would wait until all queued renames were finished -- which is exactly what your code comment said. Which, could (in theory) mean that if 20 renames were queued, even the first rename operation would have to wait for the 20th queued rename to finish. That potential worried me, of having basically a block-wait on all the renames.

With the dispatch_sync approach each rename only has to wait for the previous rename to finish.

Gavin McKenzie

On 2012-02-11, at 10:56 AM, Oliver Drobnik wrote:

Builds fine, looks more simple. I'm not that sure of myself with GCD yet, so thanks for your help!


Reply to this email directly or view it on GitHub:
#2 (comment)

Damn. I got my explanation wrong. Let me try again.

In the original implementation, additional callers would wait until all queued renames were finished -- which is exactly what your code comment said. Which, could (in theory) mean that if 20 renames were queued, even the first rename operation would have to wait for the 20th queued rename to finish. That potential worried me, of having basically a block-wait on all the renames.

With the dispatch_sync approach each rename only has to wait for the previous rename to finish.

Gavin McKenzie

On 2012-02-11, at 10:56 AM, Oliver Drobnik wrote:

Builds fine, looks more simple. I'm not that sure of myself with GCD yet, so thanks for your help!


Reply to this email directly or view it on GitHub:
#2 (comment)

odrobnik added a commit that referenced this pull request Dec 22, 2012

Merge pull request #2 from gmckenzi/patch-1
Proposed to dispatch_sync the rename, rather than waiting on a rename queue group.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment