diff --git a/AppKit/CPTableColumn.j b/AppKit/CPTableColumn.j index 1662782ee..e02c5607b 100644 --- a/AppKit/CPTableColumn.j +++ b/AppKit/CPTableColumn.j @@ -639,6 +639,53 @@ CPTableColumnUserResizingMask = 1 << 1; } } + +/*! + @ignore + + This method will return the object at a row in the first found CPArray in the key path + that is divided in a first and second part. + The first part is never a combined key path. The second part can be a combined key path. + If this optimization is not done we will create an array with the valueForKeyPath value on each row and then pick + the wanted value for the row and throw away all the other rows. It is much more effective to first + pick the row and then do the valueForKeyPath on the rest of the key path. + When the second part is depleated it will stop the search for a CPArray and return the + current object for the first part. The second part will then be nil. + The secondPartRef will always be updated with the rest of the key path that can be applied + to the returned object. + It will stop the search if the object is nil +*/ +- (CPValueCoding)_firstObjectInArrayUsingKeyPathFirstPart:(CPString)firstPart secondPart:(CPStringRef)secondPartRef sourceObject:(CPValueCoding)source forRow:(unsigned)aRow +{ + var firstValue = [source valueForKeyPath:firstPart]; + + if (firstValue == nil) + return firstValue; + + if ([firstValue isKindOfClass:CPArray]) + return [firstValue objectAtIndex:aRow]; + + var secondPart = @deref(secondPartRef); + + if (secondPart == nil) + return firstValue; + + var dotIndex = secondPart.indexOf("."); + + if (dotIndex === CPNotFound) + { + firstPart = secondPart; + @deref(secondPartRef) = nil; + } + else + { + firstPart = secondPart.substring(0, dotIndex); + @deref(secondPartRef) = secondPart.substring(dotIndex + 1); + } + + return [self _firstObjectInArrayUsingKeyPathFirstPart:firstPart secondPart:secondPartRef sourceObject:firstValue forRow:aRow]; +} + /*! @ignore */ @@ -655,7 +702,7 @@ CPTableColumnUserResizingMask = 1 << 1; bindingInfo = binding._info, destination = [bindingInfo objectForKey:CPObservedObjectKey], keyPath = [bindingInfo objectForKey:CPObservedKeyPathKey], - dotIndex = keyPath.lastIndexOf("."), + dotIndex = keyPath.indexOf("."), value; if (dotIndex === CPNotFound) @@ -670,17 +717,16 @@ CPTableColumnUserResizingMask = 1 << 1; The optimization is to get the array and access the value directly. This turns the operation into a single access regardless of how long the model - array is. + array is or how long the key path is. */ var firstPart = keyPath.substring(0, dotIndex), - secondPart = keyPath.substring(dotIndex + 1), - firstValue = [destination valueForKeyPath:firstPart]; + secondPart = keyPath.substring(dotIndex + 1); - if ([firstValue isKindOfClass:CPArray]) - value = [[firstValue objectAtIndex:aRow] valueForKeyPath:secondPart]; - else - value = [[firstValue valueForKeyPath:secondPart] objectAtIndex:aRow]; + value = [self _firstObjectInArrayUsingKeyPathFirstPart:firstPart secondPart:@ref(secondPart) sourceObject:destination forRow:aRow]; + + if (secondPart != nil) + value = [value valueForKeyPath:secondPart]; } value = [binding transformValue:value withOptions:[bindingInfo objectForKey:CPOptionsKey]]; diff --git a/Tests/AppKit/CPTableViewTest.j b/Tests/AppKit/CPTableViewTest.j index 561906603..42f82084a 100644 --- a/Tests/AppKit/CPTableViewTest.j +++ b/Tests/AppKit/CPTableViewTest.j @@ -667,6 +667,69 @@ } +/*! + Test that we don't build up an array with all the value for each key path and then pick the row and + throws away all the other values. This is performance critical as this is done for every column on every row. + + This is tested by setting up a key path to the name property that is combined. We count the number of times each + part in the key path is accessed with valueForKey. +*/ +- (void)testLazyPrepareDataView +{ + //objj_msgSend_decorate(objj_backtrace_decorator); + + var frameRect = CGRectMake(0, 0, 1024, 64); + + [theWindow setFrame:frameRect]; + + var table = [[CPTableView alloc] initWithFrame:CGRectMake(0, 0, 100, 100)], + container = [TestDataSource new]; + + var tc1 = [[CPTableColumn alloc] initWithIdentifier:@"A"]; + [table addTableColumn:tc1]; + + var tc2 = [[CPTableColumn alloc] initWithIdentifier:@"B"]; + [table addTableColumn:tc2]; + + var scrollView = [[CPScrollView alloc] initWithFrame:frameRect]; + + [[theWindow contentView] addSubview:scrollView]; + [scrollView setDocumentView:table]; + [theWindow makeFirstResponder:table]; + + var i = 1; + var dataArray = @[ + [ValueForKeyCountingDictionary dictionaryWithObject:[ValueForKeyCountingDictionary dictionaryWithObject:@{@"name":@"B"+i, @"longname": @"BB"+i++} forKey:@"b"] forKey:@"a"], + [ValueForKeyCountingDictionary dictionaryWithObject:[ValueForKeyCountingDictionary dictionaryWithObject:@{@"name":@"B"+i, @"longname": @"BB"+i++} forKey:@"b"] forKey:@"a"], + [ValueForKeyCountingDictionary dictionaryWithObject:[ValueForKeyCountingDictionary dictionaryWithObject:@{@"name":@"B"+i, @"longname": @"BB"+i++} forKey:@"b"] forKey:@"a"], + ]; + + var ac = [[CPArrayController alloc] init]; + [ac bind:@"contentArray" toObject:container withKeyPath:@"tableEntries" options:nil]; + [tc1 bind:@"value" toObject:ac withKeyPath:@"arrangedObjects.a.b.name" options:nil]; + [tc2 bind:@"value" toObject:ac withKeyPath:@"arrangedObjects.a.b.longname" options:nil]; + + [container setTableEntries:dataArray]; + + // The first two are visible so they are accessed both when setting up the observers and displaying the cell in the table view + [self assert:[[dataArray[0] countingDictionary] objectForKey:@"a"] equals:5]; + [self assert:[[dataArray[1] countingDictionary] objectForKey:@"a"] equals:5]; + // This is not visible so it is only accessed when setting up the observers + [self assert:[[dataArray[2] countingDictionary] objectForKey:@"a"] equals:2]; + + var enumerateViewsInRowsCall = 0; + + // Checks that the displayed data matches the model data. + [table enumerateAvailableViewsUsingBlock:function(dataView, aRow, aColumn, stop) + { + enumerateViewsInRowsCall++; + var data_value = [[[container tableEntries] objectAtIndex:aRow] valueForKeyPath:aColumn === 0 ? @"a.b.name" : @"a.b.longname"]; + [self assert:[dataView objectValue] equals:data_value]; + }]; + + [self assert:enumerateViewsInRowsCall equals:4]; // Only 4 views visible +} + @end @implementation FirstResponderConfigurableTableView : CPTableView @@ -752,6 +815,35 @@ @implementation CustomTextView1 : CPTextField @end +@implementation ValueForKeyCountingDictionary : CPMutableDictionary +{ + CPMutableDictionary countingDictionary @accessors; +} + +- (id)initWithObjects:(CPArray)objects forKeys:(CPArray)keyArray +{ + self = [super initWithObjects:objects forKeys:keyArray]; + if (self) { + countingDictionary = [CPDictionary new]; + } + + return self; +} + +- (id)valueForKey:(CPString)aKey +{ + var countsForKey = [countingDictionary objectForKey:aKey]; + + countsForKey = (countsForKey == nil ? 1 : countsForKey + 1); + //print([self UID] + @": valueForKey:" + aKey + ", countForKey: " + countsForKey); + //objj_backtrace_print(CPLog.error); + [countingDictionary setObject:countsForKey forKey:aKey]; + + return [super valueForKey:aKey]; +} + +@end + var getAllViews = function(aView) { var views = [aView],