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

SBJsonStreamWriter stringCache #55

Closed
stevenreinisch opened this issue Mar 17, 2011 · 4 comments
Closed

SBJsonStreamWriter stringCache #55

stevenreinisch opened this issue Mar 17, 2011 · 4 comments
Labels
Milestone

Comments

@stevenreinisch
Copy link

hi,

I am using JSON v3.0beta1 (iOS) for iOS 4.2 and have a memory issue when serializing arrays or dictionaries with (id)JSONValue. Consider the following snippet:

    NSAutoreleasePool *pool = [[NSAutoreleasePool alloc] init];
    while(TRUE)
    {
        NSString *aString = [NSString stringWithFormat:@"dummyUUID %i", rand()];
        NSArray *array = [NSArray arrayWithObjects:aString, nil];
        
        NSString *serializedArray = [array JSONRepresentation];
        
        [pool drain];
        pool = [[NSAutoreleasePool alloc] init];
    }

When running this code with Instruments and the Allocations tool there is a steep increase of live bytes that are not freed despite the autoreleasePool.

I think that the stringCache in SBJsonStreamWriter is the reason for this issue. It is created in the SBJsonStreamWriter's initialize method but never released. If I use the following macro, the steep memory increase disappears:

#define FULL_CACHE_FIXED 1

+ (void)initialize {
    notANumber = [NSDecimalNumber notANumber];
    
    #if !FULL_CACHE_FIXED
    stringCache = [NSMutableDictionary new];
    #endif
}

#pragma mark Housekeeping

- (id)init {
    self = [super init];
    if (self) {
        data = [[NSMutableData alloc] initWithCapacity:1024u];
        maxDepth = 512;
        states = calloc(maxDepth, sizeof(SBJsonStreamWriterState*));
        NSAssert(states, @"States not initialised");
        
        states[0] = [SBJsonStreamWriterStateStart sharedInstance];
        
        #if FULL_CACHE_FIXED
        stringCache = [NSMutableDictionary new];
        #endif
    }
    return self;
}

- (void)dealloc {
    self.error = nil;
    free(states);
    [data release];
    
    #if FULL_CACHE_FIXED
    [stringCache release];
    #endif
    
    [super dealloc];
}

So, allocating and releasing the stringCache for each SBJsonStreamWriter instance solves the memory issue but I did not investigate in how far performance is affected.

What was the idea behind using a static stringCache?

regards,

steven

@stig
Copy link
Collaborator

stig commented Mar 18, 2011

This cache gave quite a bit of performance when parsing typical JSON where
if you have a list of dictionaries, for example, you tend to get the same
strings (the dictionary keys) repeated again and again.

The idea behind making it static was that strings would stay cached even
across calls to the category methods, which instantiate a new parser for
each call. However, I agree it may be a bit heavy-handed. I could be swayed
to move it to the instance instead of being a static cache. Especially if it
doesn't affect performance much. Care to cook up a patch?

Stig

On 17 March 2011 22:45, stevenreinisch <
reply@reply.github.com>wrote:

hi,

I am using JSON v3.0beta1 (iOS) for iOS 4.2 and have a memory issue when
serializing arrays or dictionaries with (id)JSONValue. Consider the
following snippet:

       NSAutoreleasePool *pool = [[NSAutoreleasePool alloc] init];
       while(TRUE)
       {
               NSString *aString = [NSString stringWithFormat:@"dummyUUID
%i", rand()];
               NSArray *array = [NSArray arrayWithObjects:aString, nil];

               NSString *serializedArray = [array JSONRepresentation];

               [pool drain];
               pool = [[NSAutoreleasePool alloc] init];
       }

When running this code with Instruments and the Allocations tool there is a
steep increase of live bytes that are not freed despite the autoreleasePool.

I think that the stringCache in SBJsonStreamWriter is the reason for this
issue. It is created in the SBJsonStreamWriter's initialize method but never
released. If I use the following macro, the steep memory increase
disappears:

#define FULL_CACHE_FIXED 1

+ (void)initialize {
       notANumber = [NSDecimalNumber notANumber];

       #if !FULL_CACHE_FIXED
       stringCache = [NSMutableDictionary new];
       #endif
}

#pragma mark Housekeeping

- (id)init {
       self = [super init];
       if (self) {
               data = [[NSMutableData alloc] initWithCapacity:1024u];
               maxDepth = 512;
               states = calloc(maxDepth, sizeof(SBJsonStreamWriterState*));
               NSAssert(states, @"States not initialised");

               states[0] = [SBJsonStreamWriterStateStart sharedInstance];

               #if FULL_CACHE_FIXED
               stringCache = [NSMutableDictionary new];
               #endif
       }
       return self;
}

- (void)dealloc {
       self.error = nil;
       free(states);
       [data release];

       #if LEAK_FIXED
       [stringCache release];
       #endif

       [super dealloc];
}

So, allocating and releasing the stringCache for each SBJsonStreamWriter
instance solves the memory issue but I did not investigate in how far
performance is affected.

What was the idea behind using a static stringCache?

regards,

steven

Reply to this email directly or view it on GitHub:
#55

@stevenreinisch
Copy link
Author

I moved the cache to the instance level. But I did not measure the resulting
performance change. Is there a way to test this.

I would like to cook up a patch .. would be my first one ever :).
what exactly do I need to do to achieve this?

steven

On Mar 18, 2011, at 5:56 PM, stig wrote:

This cache gave quite a bit of performance when parsing typical JSON where
if you have a list of dictionaries, for example, you tend to get the same
strings (the dictionary keys) repeated again and again.

The idea behind making it static was that strings would stay cached even
across calls to the category methods, which instantiate a new parser for
each call. However, I agree it may be a bit heavy-handed. I could be swayed
to move it to the instance instead of being a static cache. Especially if it
doesn't affect performance much. Care to cook up a patch?

Stig

On 17 March 2011 22:45, stevenreinisch <
reply@reply.github.com>wrote:

hi,

I am using JSON v3.0beta1 (iOS) for iOS 4.2 and have a memory issue when
serializing arrays or dictionaries with (id)JSONValue. Consider the
following snippet:

      NSAutoreleasePool *pool = [[NSAutoreleasePool alloc] init];
      while(TRUE)
      {
              NSString *aString = [NSString stringWithFormat:@"dummyUUID
%i", rand()];
              NSArray *array = [NSArray arrayWithObjects:aString, nil];

              NSString *serializedArray = [array JSONRepresentation];

              [pool drain];
              pool = [[NSAutoreleasePool alloc] init];
      }

When running this code with Instruments and the Allocations tool there is a
steep increase of live bytes that are not freed despite the autoreleasePool.

I think that the stringCache in SBJsonStreamWriter is the reason for this
issue. It is created in the SBJsonStreamWriter's initialize method but never
released. If I use the following macro, the steep memory increase
disappears:

#define FULL_CACHE_FIXED 1

+ (void)initialize {
      notANumber = [NSDecimalNumber notANumber];

      #if !FULL_CACHE_FIXED
      stringCache = [NSMutableDictionary new];
      #endif
}

#pragma mark Housekeeping

- (id)init {
      self = [super init];
      if (self) {
              data = [[NSMutableData alloc] initWithCapacity:1024u];
              maxDepth = 512;
              states = calloc(maxDepth, sizeof(SBJsonStreamWriterState*));
              NSAssert(states, @"States not initialised");

              states[0] = [SBJsonStreamWriterStateStart sharedInstance];

              #if FULL_CACHE_FIXED
              stringCache = [NSMutableDictionary new];
              #endif
      }
      return self;
}

- (void)dealloc {
      self.error = nil;
      free(states);
      [data release];

      #if LEAK_FIXED
      [stringCache release];
      #endif

      [super dealloc];
}

So, allocating and releasing the stringCache for each SBJsonStreamWriter
instance solves the memory issue but I did not investigate in how far
performance is affected.

What was the idea behind using a static stringCache?

regards,

steven

Reply to this email directly or view it on GitHub:
#55

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

@stig
Copy link
Collaborator

stig commented Mar 21, 2011

Hi Steven,

Have a look at this page: http://help.github.com/fork-a-repo/

Stig

On 21 March 2011 15:39, stevenreinisch <
reply@reply.github.com>wrote:

I moved the cache to the instance level. But I did not measure the
resulting
performance change. Is there a way to test this.

I would like to cook up a patch .. would be my first one ever :).
what exactly do I need to do to achieve this?

steven

On Mar 18, 2011, at 5:56 PM, stig wrote:

This cache gave quite a bit of performance when parsing typical JSON
where
if you have a list of dictionaries, for example, you tend to get the same
strings (the dictionary keys) repeated again and again.

The idea behind making it static was that strings would stay cached even
across calls to the category methods, which instantiate a new parser for
each call. However, I agree it may be a bit heavy-handed. I could be
swayed
to move it to the instance instead of being a static cache. Especially if
it
doesn't affect performance much. Care to cook up a patch?

Stig

On 17 March 2011 22:45, stevenreinisch <
reply@reply.github.com>wrote:

hi,

I am using JSON v3.0beta1 (iOS) for iOS 4.2 and have a memory issue when
serializing arrays or dictionaries with (id)JSONValue. Consider the
following snippet:

      NSAutoreleasePool *pool = [[NSAutoreleasePool alloc] init];
      while(TRUE)
      {
              NSString *aString = [NSString stringWithFormat:@
"dummyUUID
%i", rand()];
              NSArray *array = [NSArray arrayWithObjects:aString, nil];

              NSString *serializedArray = [array JSONRepresentation];

              [pool drain];
              pool = [[NSAutoreleasePool alloc] init];
      }

When running this code with Instruments and the Allocations tool there
is a
steep increase of live bytes that are not freed despite the
autoreleasePool.

I think that the stringCache in SBJsonStreamWriter is the reason for
this
issue. It is created in the SBJsonStreamWriter's initialize method but
never
released. If I use the following macro, the steep memory increase
disappears:

#define FULL_CACHE_FIXED 1

+ (void)initialize {
      notANumber = [NSDecimalNumber notANumber];

      #if !FULL_CACHE_FIXED
      stringCache = [NSMutableDictionary new];
      #endif
}

#pragma mark Housekeeping

- (id)init {
      self = [super init];
      if (self) {
              data = [[NSMutableData alloc] initWithCapacity:1024u];
              maxDepth = 512;
              states = calloc(maxDepth,
sizeof(SBJsonStreamWriterState*));
              NSAssert(states, @"States not initialised");

              states[0] = [SBJsonStreamWriterStateStart sharedInstance];

              #if FULL_CACHE_FIXED
              stringCache = [NSMutableDictionary new];
              #endif
      }
      return self;
}

- (void)dealloc {
      self.error = nil;
      free(states);
      [data release];

      #if LEAK_FIXED
      [stringCache release];
      #endif

      [super dealloc];
}

So, allocating and releasing the stringCache for each SBJsonStreamWriter
instance solves the memory issue but I did not investigate in how far
performance is affected.

What was the idea behind using a static stringCache?

regards,

steven

Reply to this email directly or view it on GitHub:
#55

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

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

@ghost ghost assigned stig Apr 27, 2011
@stig stig closed this as completed in a93f424 Apr 27, 2011
@swisspol
Copy link

The patch as submitted is wrong: "stringCache" is never released, so effectively this fixes nothing.

I just noticed this problem while tracking some memory leaks in the framework. Someone else already reported this issue at #67.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants