Skip to content

Implement -hash and -copy of GDLLogEvent, copy on log, and don't assign extensionBytes in log writer #2204

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.

Sign up for GitHub

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

Merged
merged 5 commits into from
Dec 21, 2018
Merged

Implement -hash and -copy of GDLLogEvent, copy on log, and don't assign extensionBytes in log writer #2204

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a644257
Implement -hash and -copy of GDLLogEvent
mikehaney24 Dec 20, 2018
ad19ba3
Copy the log object upon logging
mikehaney24 Dec 20, 2018
edc586a
Don't assign extensionBytes in the log writer
mikehaney24 Dec 20, 2018
c9c7925
Make an implicit loss of precision explicit.
mikehaney24 Dec 20, 2018
77ffb0a
Add a comment on performance
mikehaney24 Dec 20, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions GoogleDataLogger/GoogleDataLogger/Classes/GDLLogEvent.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,31 @@ - (instancetype)initWithLogMapID:(NSString *)logMapID logTarget:(NSInteger)logTa
return self;
}

- (instancetype)copy {
GDLLogEvent *copy = [[GDLLogEvent alloc] initWithLogMapID:_logMapID logTarget:_logTarget];
copy.extension = _extension;
copy.extensionBytes = _extensionBytes;
copy.qosTier = _qosTier;
copy.clockSnapshot = _clockSnapshot;
return copy;
}

- (NSUInteger)hash {
// This loses some precision, but it's probably fine.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Precision should be irrelevant for a hash, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it matters in a theoretical sense, if the hash value of two times coincidentally have the same first 32 bits' worth of digits. In reality, it's not likely to matter, and I had to make this precision loss explicit in order to avoid a warning/lib lint failure.

NSUInteger timeHash = (NSUInteger)(_clockSnapshot.timeMillis ^ _clockSnapshot.uptimeMillis);
return [_logMapID hash] ^ _logTarget ^ [_extensionBytes hash] ^ _qosTier ^ timeHash;
}

- (void)setExtension:(id<GDLLogProto>)extension {
// If you're looking here because of a performance issue in -protoBytes slowing the assignment
// of extension, one way to address this is to add a queue to this class,
// dispatch_(barrier_ if concurrent)async here, and implement the getter with a dispatch_sync.
if (extension != _extension) {
_extension = extension;
_extensionBytes = [extension protoBytes];
}
}

#pragma mark - NSSecureCoding and NSCoding Protocols

/** NSCoding key for logMapID property. */
Expand Down
11 changes: 0 additions & 11 deletions GoogleDataLogger/GoogleDataLogger/Classes/GDLLogWriter.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,17 +61,6 @@ - (void)writeLog:(GDLLogEvent *)log
return;
}
}

// Convert the extension to bytes after transforming.
NSAssert([log.extension respondsToSelector:@selector(protoBytes)],
@"The log event's extension must respond to -protoBytes.");
if ([log.extension respondsToSelector:@selector(protoBytes)]) {
log.extensionBytes = [log.extension protoBytes];
log.extension = nil;
} else {
GDLLogWarning(GDLMCWExtensionMissingBytesImpl, @"%@",
@"The log event's extension must respond to -protoBytes.");
}
// TODO(mikehaney24): [[GDLLogStorage sharedInstance] storeLog:transformedLog];
});
}
Expand Down
6 changes: 4 additions & 2 deletions GoogleDataLogger/GoogleDataLogger/Classes/GDLLogger.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,12 +50,14 @@ - (instancetype)initWithLogMapID:(NSString *)logMapID

- (void)logTelemetryEvent:(GDLLogEvent *)logEvent {
NSAssert(logEvent, @"You can't log a nil event");
[[GDLLogWriter sharedInstance] writeLog:logEvent afterApplyingTransformers:_logTransformers];
GDLLogEvent *copiedLog = [logEvent copy];
[[GDLLogWriter sharedInstance] writeLog:copiedLog afterApplyingTransformers:_logTransformers];
}

- (void)logDataEvent:(GDLLogEvent *)logEvent {
NSAssert(logEvent, @"You can't log a nil event");
[[GDLLogWriter sharedInstance] writeLog:logEvent afterApplyingTransformers:_logTransformers];
GDLLogEvent *copiedLog = [logEvent copy];
[[GDLLogWriter sharedInstance] writeLog:copiedLog afterApplyingTransformers:_logTransformers];
}

- (GDLLogEvent *)newEvent {
Expand Down