NSRectArray by reference

  • I'm writing a function which given a content rect and the number of
    required rows will split it up and put the resultant rects into an
    NSRectArray provided by reference - once again my basic C lets me down.

    I'm creating the array and calling the function -

    NSRectArray rowRects = calloc(calendarInfo.totalRows, sizeof(NSRect));
    CalendarRowRects(calendarRect, calendarInfo.totalRows, &rowRects);

    The function in question -

    NS_INLINE void CalendarRowRects(NSRect calendarRect, NSUInteger rows,
    NSRectArray *rowRects) {
    CGFloat rowGapHeight = NSHeight(calendarRect)/50.0;
    CGFloat rowHeight = (NSHeight(calendarRect) - ((rows - 1) *
    rowGapHeight)) / rows;
    CGFloat delta = (rowHeight + rowGapHeight);

    NSRect currentRowRect, remainder;
    NSDivideRect(calendarRect, &currentRowRect, &remainder, rowHeight,
    NSMaxYEdge);

    for (NSUInteger index = 0; index < rows; index++) {
      (*rowRects[index]) = currentRowRect; // it crashes here (for reasons
    that are probably obvious to the reader), notably the most crucial line
      currentRowRect.origin.y += delta;
    }
    }

    Granted that letting the function assume there is a enough space in
    the provided array isn't a good design decision but this is a private
    internal function that is only going to be called in -drawRect and -
    mouseDown: so I can get away with it.

    I could wrap the rects in NSValues and return an NSArray but I don't
    really want that overhead, I always try to avoid using Obj-C in my
    drawing code as possible.

    - Keith
  • > Granted that letting the function assume there is a enough space in
    > the provided array isn't a good design decision but this is a
    > private internal function that is only going to be called in -
    > drawRect and -mouseDown: so I can get away with it.

    Nevermind, I've become a little obsessed with passing and returning
    arguments by reference lately. I've renamed the function to
    CreateCalendarRowRects and simply malloc'ing the memory in there then
    returning the array pointer.

    - Keith
  • On 09.09.2007, at 19:45, Keith Duncan wrote:
    > (*rowRects[index]) = currentRowRect; // it crashes here (for
    > reasons that are probably obvious to the reader), notably the most
    > crucial line
    > currentRowRect.origin.y += delta;

      This is kind of a short, and probably confusing explanation, but I
    don't want to post the entire contents of my explanation of pointers
    here, you'll want to read them with all the pretty pictures at
    <http://www.zathras.de/howmemorymanagementworks.htm>. You may also
    want to read the "Masters of the Void" C tutorial that's linked on
    that page.

      A pointer is simply a number, an address. An array is simply a
    pointer, just that it points to a bunch of elements instead of one
    (which you probably know, since you're using calloc() to create a new
    block and return its address). It logically follows that an
    NSRectArray is simply a variable containing a number (an address).

      You do not want to change this number, so why are you passing a
    pointer to this number into your function? Just pass in the address
    by value. While it will copy the address, it will not copy the memory
    block pointed to by this address, so you're effectively passing by
    reference anyway.

      Your problem above is probably one of precedence. What your code
    does is get the index-th element of an array of NSRect*s, and then de-
    reference that. While what you really want to do is first dereference
    your NSRect** so you can then get the index-th element from it. To do
    that, you'd probably want to say:

    ((*rowRects)[index]) = currentRowRect;

    When in doubt, it's always a good idea to use brackets to explicitly
    enforce execution order, instead of hoping the computer will do what
    you intended it to do. :-)

    Cheers,
    -- M. Uli Kusterer
    http://www.zathras.de
  • On 09.09.2007, at 19:45, Keith Duncan wrote:
    > NS_INLINE void CalendarRowRects(NSRect calendarRect, NSUInteger
    > rows, NSRectArray *rowRects) {

      Oh, and by the way: If you have problems with by-reference passing,
    you shouldn't be playing with keywords like NS_INLINE. Inlining is a
    performance optimization that causes the function body to be "copied
    in" wherever it is called, instead of having one central function and
    jumping to that and back. In most cases, what it actually causes to
    happen is that the size of your program unnecessarily grows in size
    (because you have dozens of copies of the same function instead of
    just one), and that in turn will make your app use more RAM, and will
    cause parts of it to be swapped to disk and read from disk again. And
    when your app swaps, it's a much bigger performance drain than
    function call overhead.

      In summary: Don't use inlining unless you know what it does, and
    run your application in the profiler (Shark) and determine whether
    function call overhead is really what's slowing things down.
    Generally, a function with a loop in it will be called rarely and
    will spend most of its time in its loop, so inlining won't help.
    Generally, mostly small functions benefit from inlining, and here
    small usually implies no other function calls.

      In general the rule is: Profile first, and then optimize the places
    you spend the most time in. And that's generally functions you call
    repeatedly. What use is it shaving a billionth of a second off a
    function that is called only once over the life of your application?
    Nobody will ever notice.

    Cheers,
    -- M. Uli Kusterer
    http://www.zathras.de
  • You have an extra dereference is several places.  To fix see 3 notes
    below

    > CalendarRowRects(calendarRect, calendarInfo.totalRows,
    > &rowRects);    // note: remove the &
    >
    > The function in question -
    >
    > NS_INLINE void CalendarRowRects(NSRect calendarRect, NSUInteger
    > rows, NSRectArray *rowRects) {  // note: remove the *
    > (*rowRects[index]) = currentRowRect; // note: remove the outer
    > parens and the *

    > Nevermind, I've become a little obsessed with passing and returning
    > arguments by reference lately. I've renamed the function to
    > CreateCalendarRowRects and simply malloc'ing the memory in there
    > then returning the array pointer.

    The problem with this solution is that you will be mixing Obj-C
    memory management policies with your ad hoc C/malloc memory policy.
    While it is certainly possible to do this correctly it may be
    confusing to someone who looks at the code in a year or two (even
    you). It may result in a memory leak.

    A few suggestions:

    * Use the NSArray solution that you have rejected. You didn't mention
    how many rows there are but if it's less than say 100 there will not
    be much overhead.

    * Use an NSMutableData instance to hold the rects. There will be less
    Obj-C overhead in this case than the NSArray solution.

    * Make the data block that holds the NSRects a data member and make
    the CalendarRowRects function a method.  Reallocate the memory block
    as required each time the CalendarRowRects method is called. Release
    the memory block in your object's dealloc method. This has the
    benefit of being very simple and straightforward and makes memory
    errors unlikely. It will also have relatively low overhead.

    --
    Brian Stern
    <brians99...>
  • On 09.09.2007, at 23:19, Brian Stern wrote:
    > * Use the NSArray solution that you have rejected. You didn't
    > mention how many rows there are but if it's less than say 100 there
    > will not be much overhead.
    >
    > * Use an NSMutableData instance to hold the rects. There will be
    > less Obj-C overhead in this case than the NSArray solution.
    >
    > * Make the data block that holds the NSRects a data member and make
    > the CalendarRowRects function a method.  Reallocate the memory
    > block as required each time the CalendarRowRects method is called.
    > Release the memory block in your object's dealloc method. This has
    > the benefit of being very simple and straightforward and makes
    > memory errors unlikely. It will also have relatively low overhead.

      There's always an option "D". Even when using bullet points in the
    mistaken assumption that an option "D" can only exist when there are
    options "A" through "C", and thus I could be prevented from claiming
    there was an option "D". :-p

      D) Create your own KDRectArray class that internally uses calloc()/
    free() to create/dispose of its memory block, but wraps it all nicely
    in an Objective C object that everyone can use, including indexed
    accessors like:

    -(NSRect) rectAtIndex: (unsigned)idx;
    -(void) setRect: (NSRect)box atIndex: (unsigned)idx;

    And you could even go overboard and provide objectAtIndex/
    setObject:atIndex:, which would return an NSValue containing a rect
    or whatever, and could be used with other classes that expect an
    NSArray, not a KDRectArray. Well, if you're careful, at least.

    Cheers,
    -- M. Uli Kusterer
    http://www.zathras.de
  • On 9 Sep 2007, at 22:28, Uli Kusterer wrote:

    > D) Create your own KDRectArray class that internally uses calloc()/
    > free() to create/dispose of its memory block, but wraps it all
    > nicely in an Objective C object that everyone can use, including
    > indexed accessors like:

    IMHO if you're doing that, it's worth considering using an
    NSMutableData as the backing store (inside your KDRectArray object),
    because that strategy will work better with garbage collected code.
    (The existence of GC is something that's been public for some time,
    so I don't think I'm revealing anything "secret" by saying that; I'm
    not sure I can explain *why* using NSMutableData might be better
    though :-).)

    Kind regards,

    Alastair.

    --
    http://alastairs-place.net
  • On 10.09.2007, at 12:59, Alastair Houghton wrote:
    > On 9 Sep 2007, at 22:28, Uli Kusterer wrote:
    >> D) Create your own KDRectArray class that internally uses calloc()/
    >> free() to create/dispose of its memory block, but wraps it all
    >> nicely in an Objective C object that everyone can use, including
    >> indexed accessors like:
    >
    > IMHO if you're doing that, it's worth considering using an
    > NSMutableData as the backing store (inside your KDRectArray
    > object), because that strategy will work better with garbage
    > collected code.  (The existence of GC is something that's been
    > public for some time, so I don't think I'm revealing anything
    > "secret" by saying that; I'm not sure I can explain *why* using
    > NSMutableData might be better though :-).)

      I'm not garbage-collected yet, so my stance on this is: Let's burn
    that bridge when we get to it ;-)

      The disadvantage of using NSData as a replacement for malloc in
    cases like these is that you always need to do ((NSRect*)[myData
    mutableBytes]) when you want to access the array, instead of just
    going through an NSRect* right away. Of course, you could add a
    second ivar, but really, that would be kind of wasteful.

      I'm not quite sure why it would be a problem to use malloc, though.
    After all, most Apple classes will eventually be doing that
    internally, garbage collected or not, so this problem has to have
    been solved already, if it even is one. Of course, in garbage
    collected systems you're never supposed to rely on memory actually
    going away when you think, but as long as the internal storage gets
    freed when the object is reclaimed, who cares whether that's now or
    five minutes from now during the next GC cycle.

    Cheers,
    -- M. Uli Kusterer
    http://www.zathras.de
previous month september 2007 next month
MTWTFSS
          1 2
3 4 5 6 7 8 9
10 11 12 13 14 15 16
17 18 19 20 21 22 23
24 25 26 27 28 29 30
Go to today