Added a lock around newMatchingFont #63

Merged
merged 4 commits into from Aug 23, 2011

Conversation

Projects
None yet
2 participants
@dhoerl
Contributor

dhoerl commented Aug 17, 2011

Prevents multiple threads from trying to instantiate the same new font

@odrobnik

This comment has been minimized.

Show comment
Hide comment
@odrobnik

odrobnik Aug 21, 2011

Collaborator

Why do you need to force the font creation onto main thread? Isn't that causing more of a bottleneck than necessary?

Collaborator

odrobnik commented Aug 21, 2011

Why do you need to force the font creation onto main thread? Isn't that causing more of a bottleneck than necessary?

@dhoerl

This comment has been minimized.

Show comment
Hide comment
@dhoerl

dhoerl Aug 21, 2011

Contributor

On 8/21/11 8:29 AM, Cocoanetics wrote:

Why do you need to force the font creation onto main thread? Isn't that causing more of a bottleneck than necessary?

Because other code on the main thread may also be calling the CFFont
methods. You cannot assume that its just your framework doing it. [That
is, the loading of various nibs maybe be generating these calls too
since some of the GUI elements use fonts in the app bundle.

I've been using Grand Central Dispatch more and more, and I think it
would be better to use the dispatch_main_queue for this and just add the
method call synchronously.

Right now my iPhone code is spawning 4 threads that grab a web page,
call this code in your framework. and return. Since adding the lock it
apparently is running faster, and frankly I cannot visually see any
delays or stuttering.

David

Contributor

dhoerl commented Aug 21, 2011

On 8/21/11 8:29 AM, Cocoanetics wrote:

Why do you need to force the font creation onto main thread? Isn't that causing more of a bottleneck than necessary?

Because other code on the main thread may also be calling the CFFont
methods. You cannot assume that its just your framework doing it. [That
is, the loading of various nibs maybe be generating these calls too
since some of the GUI elements use fonts in the app bundle.

I've been using Grand Central Dispatch more and more, and I think it
would be better to use the dispatch_main_queue for this and just add the
method call synchronously.

Right now my iPhone code is spawning 4 threads that grab a web page,
call this code in your framework. and return. Since adding the lock it
apparently is running faster, and frankly I cannot visually see any
delays or stuttering.

David

@odrobnik

This comment has been minimized.

Show comment
Hide comment
@odrobnik

odrobnik Aug 22, 2011

Collaborator

That does not explain the performing on main thread. The mutex should be sufficient, shouldn't it?

Collaborator

odrobnik commented Aug 22, 2011

That does not explain the performing on main thread. The mutex should be sufficient, shouldn't it?

@dhoerl

This comment has been minimized.

Show comment
Hide comment
@dhoerl

dhoerl Aug 22, 2011

Contributor

I started two emails to you on this topic, and will try to glue them
together as I discovered more information.


Sorry, perhaps I did not make myself clear. I do not believe that the
CGFont... routines are thread safe. I don't know this for a fact but
suspect it.

If that is true, then my issue is that other code on my main thread -
nib loading routines performed by the system - may also be executing the
CGFont... code.

By running your class usage on the main thread, this serializes both all
instances of your class with any other usages by the system (or even the
user).

The mutex would help serialize your class' usage of its font cache, but
does not address the above concern.

Every crash I got was in the system code - not your code. I can send you
other crash logs if you like.


I'm doing an app for my employer, who is not going to accept even one
crash by one user. Since this crash has been so hard to track down for
me, I want to err on the side of caution and not performance.

You already have a flag for thread-safe - you could add another one for
font-thread-safety. That said, most users won't have this problem as
they probably are not initializing a whole bunch of instances on
different threads at one time.


OK - I just did more homework on this. Apple claims all of CoreText is
threadsafe. So, this implies that it should be sufficient to just lock
modification of the library font cache. But...

I thought - maybe I can cause this by pounding the system - and yes - I
can force the crash at every startup. I put the following code below in
my applicationDidStart. It causes 80 concurrent actions - and it crashes
every time. Lower the number to 8 and it does not crash. There is some
race condition in Apple's code. Going to generate a bug report in a few
minutes - will send you the text project if you want.


So, in the end - yes - you need to serialize these calls on the main
thread - at least for the iphone in iOS 5 or older.

David

CODE:

{
#import <CoreText/CoreText.h>

dispatch_queue_t q = 

dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0);

for(int i=0; i<80; ++i) {
    typedef void (^foo)(void);
    foo f;
    if((i % 4) == 0)
        f = ^
                {
                    CGFloat pointSize = 28;
                    NSDictionary *dict = [NSDictionary dictionaryWithObjectsAndKeys:
                            @"Arvo", @"NSFontFamilyAttribute",
                            [NSNumber numberWithInt:2], @"NSCTFontSymbolicTrait",
                            [NSNumber numberWithFloat:28], @"NSFontSizeAttribute",
                        nil];
                    CTFontDescriptorRef fontDesc = 

CTFontDescriptorCreateWithAttributes((__bridge CFDictionaryRef)dict);
CTFontCreateWithFontDescriptor(fontDesc, pointSize, NULL);
};
if((i % 4) == 1)
f = ^
{
CGFloat pointSize = 28;
NSDictionary *dict = [NSDictionary dictionaryWithObjectsAndKeys:
@"Arvo", @"NSFontFamilyAttribute",
[NSNumber numberWithFloat:14], @"NSFontSizeAttribute",
nil];
CTFontDescriptorRef fontDesc =
CTFontDescriptorCreateWithAttributes((__bridge CFDictionaryRef)dict);
CTFontCreateWithFontDescriptor(fontDesc, pointSize, NULL);
};
if((i % 4) == 2)
f = ^
{
CGFloat pointSize = 28;
NSDictionary *dict = [NSDictionary dictionaryWithObjectsAndKeys:
@"Arvo", @"NSFontFamilyAttribute",
[NSNumber numberWithFloat:16.379999], @"NSFontSizeAttribute",
nil];
CTFontDescriptorRef fontDesc =
CTFontDescriptorCreateWithAttributes((__bridge CFDictionaryRef)dict);
CTFontCreateWithFontDescriptor(fontDesc, pointSize, NULL);
};
if((i % 4) == 3)
f = ^
{
CGFloat pointSize = 28;
NSDictionary *dict = [NSDictionary dictionaryWithObjectsAndKeys:
@"Arvo", @"NSFontFamilyAttribute",
[NSNumber numberWithInt:2], @"NSCTFontSymbolicTrait",
[NSNumber numberWithFloat:16.379999], @"NSFontSizeAttribute",
nil];
CTFontDescriptorRef fontDesc =
CTFontDescriptorCreateWithAttributes((__bridge CFDictionaryRef)dict);
CTFontCreateWithFontDescriptor(fontDesc, pointSize, NULL);
};

    dispatch_async(q, f);
}

}

Contributor

dhoerl commented Aug 22, 2011

I started two emails to you on this topic, and will try to glue them
together as I discovered more information.


Sorry, perhaps I did not make myself clear. I do not believe that the
CGFont... routines are thread safe. I don't know this for a fact but
suspect it.

If that is true, then my issue is that other code on my main thread -
nib loading routines performed by the system - may also be executing the
CGFont... code.

By running your class usage on the main thread, this serializes both all
instances of your class with any other usages by the system (or even the
user).

The mutex would help serialize your class' usage of its font cache, but
does not address the above concern.

Every crash I got was in the system code - not your code. I can send you
other crash logs if you like.


I'm doing an app for my employer, who is not going to accept even one
crash by one user. Since this crash has been so hard to track down for
me, I want to err on the side of caution and not performance.

You already have a flag for thread-safe - you could add another one for
font-thread-safety. That said, most users won't have this problem as
they probably are not initializing a whole bunch of instances on
different threads at one time.


OK - I just did more homework on this. Apple claims all of CoreText is
threadsafe. So, this implies that it should be sufficient to just lock
modification of the library font cache. But...

I thought - maybe I can cause this by pounding the system - and yes - I
can force the crash at every startup. I put the following code below in
my applicationDidStart. It causes 80 concurrent actions - and it crashes
every time. Lower the number to 8 and it does not crash. There is some
race condition in Apple's code. Going to generate a bug report in a few
minutes - will send you the text project if you want.


So, in the end - yes - you need to serialize these calls on the main
thread - at least for the iphone in iOS 5 or older.

David

CODE:

{
#import <CoreText/CoreText.h>

dispatch_queue_t q = 

dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0);

for(int i=0; i<80; ++i) {
    typedef void (^foo)(void);
    foo f;
    if((i % 4) == 0)
        f = ^
                {
                    CGFloat pointSize = 28;
                    NSDictionary *dict = [NSDictionary dictionaryWithObjectsAndKeys:
                            @"Arvo", @"NSFontFamilyAttribute",
                            [NSNumber numberWithInt:2], @"NSCTFontSymbolicTrait",
                            [NSNumber numberWithFloat:28], @"NSFontSizeAttribute",
                        nil];
                    CTFontDescriptorRef fontDesc = 

CTFontDescriptorCreateWithAttributes((__bridge CFDictionaryRef)dict);
CTFontCreateWithFontDescriptor(fontDesc, pointSize, NULL);
};
if((i % 4) == 1)
f = ^
{
CGFloat pointSize = 28;
NSDictionary *dict = [NSDictionary dictionaryWithObjectsAndKeys:
@"Arvo", @"NSFontFamilyAttribute",
[NSNumber numberWithFloat:14], @"NSFontSizeAttribute",
nil];
CTFontDescriptorRef fontDesc =
CTFontDescriptorCreateWithAttributes((__bridge CFDictionaryRef)dict);
CTFontCreateWithFontDescriptor(fontDesc, pointSize, NULL);
};
if((i % 4) == 2)
f = ^
{
CGFloat pointSize = 28;
NSDictionary *dict = [NSDictionary dictionaryWithObjectsAndKeys:
@"Arvo", @"NSFontFamilyAttribute",
[NSNumber numberWithFloat:16.379999], @"NSFontSizeAttribute",
nil];
CTFontDescriptorRef fontDesc =
CTFontDescriptorCreateWithAttributes((__bridge CFDictionaryRef)dict);
CTFontCreateWithFontDescriptor(fontDesc, pointSize, NULL);
};
if((i % 4) == 3)
f = ^
{
CGFloat pointSize = 28;
NSDictionary *dict = [NSDictionary dictionaryWithObjectsAndKeys:
@"Arvo", @"NSFontFamilyAttribute",
[NSNumber numberWithInt:2], @"NSCTFontSymbolicTrait",
[NSNumber numberWithFloat:16.379999], @"NSFontSizeAttribute",
nil];
CTFontDescriptorRef fontDesc =
CTFontDescriptorCreateWithAttributes((__bridge CFDictionaryRef)dict);
CTFontCreateWithFontDescriptor(fontDesc, pointSize, NULL);
};

    dispatch_async(q, f);
}

}

@dhoerl

This comment has been minimized.

Show comment
Hide comment
@dhoerl

dhoerl Aug 22, 2011

Contributor

On 8/22/11 10:49 AM, Cocoanetics wrote:

That does not explain the performing on main thread. The mutex should be sufficient, shouldn't it?

Well, this is not going to be fun (for me). I did a test project - using
the GCD concurrent blocks - and even at a 1000 done over and over - no
crash. There is obviously something else going on - perhaps some other
CT function is involved.

Contributor

dhoerl commented Aug 22, 2011

On 8/22/11 10:49 AM, Cocoanetics wrote:

That does not explain the performing on main thread. The mutex should be sufficient, shouldn't it?

Well, this is not going to be fun (for me). I did a test project - using
the GCD concurrent blocks - and even at a 1000 done over and over - no
crash. There is obviously something else going on - perhaps some other
CT function is involved.

odrobnik added a commit that referenced this pull request Aug 23, 2011

Merge pull request #63 from dhoerl/master
This adds locking around creating a new font to make initWithHTML work on multiple threads. >= 4.3 it uses GCD, otherwise a mutex.

@odrobnik odrobnik merged commit 078ef84 into Cocoanetics:master Aug 23, 2011

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment