0

I have two arrays: array1 and array2. Each object of arrays is an array too (2D arrays). In this way I multiple them. So how I have big arrays I use dispatch_apply. Every time i receive different results include a right result. Maybe somebody knows how to fix it?

    dispatch_apply([array2 count], queue, ^(size_t j)
    {
        k = 0;
        for (int l = 0; l < [[array1 objectAtIndex:0] count]; l++) {
            k += [[[array1 objectAtIndex:i] objectAtIndex:l] intValue] *
                   [[[array2 objectAtIndex:j] objectAtIndex:l] intValue];
        }
        kNSNumber = [NSNumber numberWithInt:k];
        [multipliedArrayInto replaceObjectAtIndex:j withObject:kNSNumber];
    });
    [resulArray insertObject:multipliedArrayInto atIndex:i];
}
Brian Sachetta
  • 3,319
  • 2
  • 34
  • 45
  • Where does 'i' come from in your example? Other than that, this looks to be chock-full of race conditions. Try a for or for-each loop, no real need to parallelize this kind of task. – diatrevolo Apr 15 '15 at 15:51
  • 1
    Please, please, please throw your Objective-C code away and do this in plain old C. This is not something that should be done in Objective-C. Your code will run about a factor 1,000 slower than it should. And think about dispatch_apply being concurrent and NSMutableArray not being thread safe and what the consequences of this are. – gnasher729 Apr 15 '15 at 17:12

2 Answers2

1

There's two things, I can suggest, and I bet one of them (or both) is the overarching solution to your problem.

First, I would declare k local to the block, so there would be no question that you are overwriting it or not. You likely have the same problem with kNSNumber inside the block. If you are just using that NSNumber instance to slam into the multipliedArrayInto accumulator, you may as well remove kNSNumber, and use @(k) in it's place (if only to be more readable). Similarly, make sure multipliedArrayInto is declared just before the dispatch_apply, in what looks like an outer for loop (where ever i is coming from). And finally, make sure resulArray is instantiated, or otherwise readied just before that outer for loop.

Second, is queue a concurrent or serial queue? If you are using dispatch_apply like a parallel-executing for/enumeration -- which is likely, I think, so you are taking about handling "big arrays" efficiently -- then you are practically guaranteeing that k is being overwritten. If you change it to serial, it may work as designed. If you want it to be parallel, you will need to move the declaration of your k accumulator inside the block, and make sure the declaration of other variables makes sense, too.

Update to reflect question updates:

@antonytonies ideally, your followup answer on this thread should be moved into the question itself, so that people can follow this thread easier.

So, it looks like what I described is exactly your problem.

The global queues are all concurrent queues, which means that (hypothetically) all the dispatch blocks are executing at once, and the contents of k and other variables are getting blown away depending on how the order of the blocks executes.

I've taken your update (in the "answer" you added), and modified it to probably work:

// I renamed your method, because nameless parameters pain me. This is cosmetic, and doesn't
// matter for the problem at hand.
- (NSMutableArray *)multiplicationArrays:(NSMutableArray *)array vector:(NSMutableArray *)vector
{
  // IMHO, you want to set resultArray to nil here. Another option is to set it to nil in the
  // else case, below. Properties in Objective-C are initalized to nil,0,false,etc; you can
  // rely on ARC to initialize pointer to objc objects on the stack, too. However, someone
  // reading this code may or may not know that. IMHO, using the explicitly assignement makes it
  // clear that you're going to be returning `nil` or an instance of `NSMutableArray`.
  NSMutableArray *resultArray = nil;

  if ([[array objectAtIndex:0] count] == [vector count]) {

    // Nicely done w/ pre-allocating the result array here, so that there's no question
    // of the indexes matches the results later on.
    resultArray = [[NSMutableArray alloc] initWithCapacity:[array count]];
    for (int i=0; i < [array count]; i++) {
      [resultArray insertObject:[NSNull null] atIndex:i];
    }

    // 'queue' here is a concurrent queue. This means that you are proclaiming to the runtime
    // that the blocks being executed are able to operate correctly w/o interference from each
    // other. This is also thought of in terms of parallel execution: all these blocks may run
    // **at once**. This *also* means, that you must not share storage between them.
    dispatch_queue_t queue = dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0);

    dispatch_apply([array count], queue, ^(size_t j) {
      // Moved 'result' inside the block.
      NSInteger result = 0;

      for (int l = 0; l < [[array objectAtIndex:0] count]; l++) {
        // These array reads are **NOT** thread safe. They probably don't cause must trouble in
        // practice, but you may want to reconfigure this.
        result += [[[array objectAtIndex:j] objectAtIndex:l] intValue] * [[vector objectAtIndex:l] intValue];
      }

      // The replace of the object into resultArray is **NOT** thread-safe.
      // This probably hasn't caused you much trouble, since you can guarantee that
      // you aren't writing at the same index. However, I would strongly suggest to
      // change this to be thread-safe.
      [resultArray replaceObjectAtIndex:j withObject:@(result)];
    });

  }
  else {
    NSLog(@"matrix count isn't correspond");
  }

  return resultArray;
}

Finally: consider just using Apple's Accelerate framework for this sort of problem solving. It's available on OSX and iOS, so you should have all of your bases covered.

greymouser
  • 3,133
  • 19
  • 22
  • I declared `k, multipliedArrayInto, resultArray` before to the block. And my queue is `dispatch_queue_t queue = dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0);` – antony tonies Apr 15 '15 at 16:33
  • it's the same thing if I multiple 2D-array and vector – antony tonies Apr 15 '15 at 16:48
  • @antonytonies I updated my answer, using your second post, to explain the minimum changes you must make. As I noted, this still won't make your code 100% thread-safe. – greymouser Apr 15 '15 at 20:41
0

it's the same thing if I multiple 2D-array and vector

-(NSMutableArray*)multiplicationArraysWithVector:(NSMutableArray *)array :(NSMutableArray *)vector
{
    NSMutableArray* resultArray;

    if ([[array objectAtIndex:0] count] == [vector count])
         {
             resultArray = [[NSMutableArray alloc] initWithCapacity:[array count]];
             for (int i=0; i < [array count]; i++) {
                 [resultArray insertObject:[NSNull null] atIndex:i];
             }

              __block NSInteger result;

             dispatch_queue_t queue = dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0);
             dispatch_apply([array count], queue, ^(size_t j)
                            {
                                result = 0;


                                for (int l = 0; l < [[array objectAtIndex:0] count]; l++) {
                                    result += [[[array objectAtIndex:j] objectAtIndex:l] intValue] * [[vector objectAtIndex:l]intValue];
                                }

                                [resultArray replaceObjectAtIndex:j withObject:@(result)];
                            });


    }
    else
    {
        NSLog(@"matrix count isn't correspond");
    }

    return  resultArray;

}

In this case I can get a right or wrong data result.