jeffh / Hydrant

A simple data mapper / object serializer for objective-c
BSD 3-Clause "New" or "Revised" License
18 stars 3 forks source link

Performance improvement and easy of use. #36

Open alexlee002 opened 6 years ago

alexlee002 commented 6 years ago

Hi, I read the doc and source code, Hydrant is a good design framework! It's a good model to programmers to learn how to design a program.

But I also found some small issues:

jeffh commented 6 years ago

Hey Alex,

I appreciate the kind words 😊.

I think you might be looking for HYDMapReflectively http://hydrant.readthedocs.io/en/latest/mapper_reference.html#hydreflectivemapper which uses reflection to determine how to map fields by its type. The reflective mapper can be configured for various custom scenarios:

Custom key transformation (eg - snake_case to camelCase) Specific overrides of fields Specific overrides by type

I don’t know if it covers all the types you need by default https://github.com/jeffh/Hydrant/blob/aff5b3e1cdd94736d29f6ec738b5ab5b09c01d19/Hydrant/Public/Mappers/Facade/HYDReflectiveMapper.m#L70-L75, but let me know.

As for KVC, it is the most compatible since that’s what was easier to implement and the most straightforward to explain to Objective-C developers. IIRC, keypaths also allow updating of specific indices in arrays, which comes from past experience of interacting with APIs that have JSON “tuples” (fixed-sized arrays).

The goal of Hydrant leads more toward the safe and flexible side than the performant side (the thought being: if you really needed maximum performance, you could already hand-write all the mappings).

That being said, I’m totally open to other implementations. Hydrant gives you the ability to write custom implementations of HYDAccessor https://github.com/jeffh/Hydrant/blob/master/Hydrant/Public/Accessors/HYDAccessor.h to achieve alternative implementations from what Hydrant comes with http://hydrant.readthedocs.io/en/latest/accessor_reference.html. In short, an access describes how Hydrant can read a value from an object. Anytime you see a key path string in a mapper, you can technically replace it with any object that implements that protocol. It makes it useful to test against the current Hydrant implementation.

I’ll admit that I haven’t as thoroughly designed the HYDAccessor protocol as HYDMapper, but let me know what you think.

Cheers, Jeff

PS - The majority of Hydrant’s code avoids caching since it was built in a multi-threaded project. It was just simpler to avoid state mutation as much as possible to keep thread safety.

Sent from my iPhone

On Jul 7, 2018, at 7:02 AM, Alex Lee notifications@github.com wrote:

Hi, I read the doc and source code, Hydrant is a good design framework! It's a good model to programmers to learn how to design a program.

But I also found some small issues:

It's not very convenient to developers to use. (Here is another JSON mapping framework that it's easy to use: YYModel https://github.com/ibireme/YYModel). If the name of object's property is equals to the JSON's keyword, the framework will automatically mapping it, AND, most of the type (eg: NSString, cstring, Number, int, float, double, NSData, char *, NSDate, NSURL etc) will be automatically transformed. The KVC mapping performance: Yes, KVC is the most compatible transformer, but it waste lots of time in message forward. could you consider use objc_msgSend or IMP directly? and cache some most used mapper? — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/jeffh/Hydrant/issues/36, or mute the thread https://github.com/notifications/unsubscribe-auth/AAEMCG7jHsvQyr47TYARmPhkIKfF2pnMks5uEL-QgaJpZM4VGXqh.

alexlee002 commented 6 years ago

Thanks JeffH,

Here is my test:

@interface HYDObj1: NSObject
@property(nonatomic, assign)    NSInteger   intVal;
@property(nonatomic, assign)    float       floatVal;
@property(nonatomic, copy)      NSString    *str;
@property(nonatomic, strong)    NSURL       *url;
@property(nonatomic, strong)    NSDate      *date;
@end

    NSDictionary *json = @{
                            @"int_val": @(123), @"float_val": @(12.34), @"str": @"this is a string",
                            @"url": @"http://www.baidu.com", @"date": @"2018-07-05 00:00:00"
                            };

    CFTimeInterval t = CFAbsoluteTimeGetCurrent();
    for (int i = 0; i < 10000; ++i) {
        [HYDObj1 yy_modelWithJSON:json];
    }
    NSLog(@"YYModel: %.4f", CFAbsoluteTimeGetCurrent() - t);

    id<HYDMapper> mapper = HYDMapReflectively(HYDObj1.class).withNoRequiredFields.customMapping(@{@"int_val": @"intVal", @"float_val": @"floatVal"});
    HYDError *error = nil;
    t = CFAbsoluteTimeGetCurrent();
    for (int i = 0; i < 10000; ++i) {
        [mapper objectFromSourceObject:json error:&error];
    }
    NSLog(@"Hydrant: %.4f", CFAbsoluteTimeGetCurrent() - t);

the result is:

HydrantTest[48230:2407977] YYModel: 0.5854
HydrantTest[48023:2391766] Hydrant: 71.5280

It seems the performance is an issue?

jeffh commented 6 years ago

Hey Alex,

Sorry if I wasn’t clear, but yes, it can be improved. I’m not saying it’s not an issue (it is). If I happen to get some feel time to debug it I’ll get around to fix it (unfortunately, it’s in a long list of things I need to do in my spare time).

Hydrant’s design originates from a different problem that typical JSON to Object mappers: a hostile API being read where correct parsing behavior is difficult to accomplish.

Trying to interpret JSON like where elements can be any unexpected value. For example: [{“a”: 1}, {“b”: 2}, null, true, 0] [{“a”: 1}…] but map to two different classes based on keys in each element {“number”: X} where X can be Integers (eg - 1) Floats (eg - 1.0) Booleans (eg - true) Strings (eg - “1.0” should be 1.0) Strings that aren’t just numbers (eg - “$1.00” should be 1.0) Null A string with contents of null (eg - “null”) An empty array An array with one value of any of the above values An array with two of any of the above values if there’s two, then they need to be mapped to separate fields Also parsing tuple-like structures: {“name”: [“John”, “Doe”]} to map to givenName and familyName fields on the object But sometimes the JSON is {“name”: “John Doe”}... Produces error message that can allow you to diagnose why an JSON blob failed to parse that only occurs in production, but may be too sensitive (or expensive) to send the JSON blob over the wire to a metrics collector.

Sadly, these cases are from actual APIs I’ve interacted with that could change/break at any time. Hydrant is attempting to cover the worst case scenario of API parsing. I haven’t looked much into YYModel but I’m guessing it's generally assuming the most straight-forward case (set as nil or error).

I looked though the apple docs and definitely there’s stuff Hydrant can change:

Various APIs like NSDateFormatter weren’t thread safe to use during its development. Updating Hydrant to cache those instances would help. IIRC, Hydrant spends a non-trival amount of time generating debugging information when mapping fails. Cutting/limiting that feature can speed it up. It could probably duplicate/inline more of it’s code for performance, but that might be a tradeoff of maintainability or flexibility.

That being said, the majority of the time another JSON mapper is a better solution. Most apps don’t need this crazy level of mapping.

I hope that’s a bit more clear.

Thanks, Jeff

On Jul 8, 2018, at 11:13 PM, Alex Lee notifications@github.com wrote:

Thanks JeffH,

Here is my test:

@interface HYDObj1: NSObject @property(nonatomic, assign) NSInteger intVal; @property(nonatomic, assign) float floatVal; @property(nonatomic, copy) NSString str; @property(nonatomic, strong) NSURL url; @property(nonatomic, strong) NSDate *date; @end

NSDictionary *json = @{
                        @"int_val": @(123), @"float_val": @(12.34), @"str": @"this is a string",
                        @"url": @"http://www.baidu.com", @"date": @"2018-07-05 00:00:00"
                        };

CFTimeInterval t = CFAbsoluteTimeGetCurrent();
for (int i = 0; i < 10000; ++i) {
    [HYDObj1 yy_modelWithJSON:json];
}
NSLog(@"YYModel: %.4f", CFAbsoluteTimeGetCurrent() - t);

id<HYDMapper> mapper = HYDMapReflectively(HYDObj1.class).withNoRequiredFields.customMapping(@{@"int_val": @"intVal", @"float_val": @"floatVal"});
HYDError *error = nil;
t = CFAbsoluteTimeGetCurrent();
for (int i = 0; i < 10000; ++i) {
    [mapper objectFromSourceObject:json error:&error];
}
NSLog(@"Hydrant: %.4f", CFAbsoluteTimeGetCurrent() - t);

the result is:

HydrantTest[48230:2407977] YYModel: 0.5854 HydrantTest[48023:2391766] Hydrant: 71.5280 It seems the performance is an issue?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/jeffh/Hydrant/issues/36#issuecomment-403370576, or mute the thread https://github.com/notifications/unsubscribe-auth/AAEMCInnuj3kP6GBMbKp1lji6FZ5H6XOks5uEvSigaJpZM4VGXqh.

alexlee002 commented 6 years ago

Thanks @jeffh.

These days I try to use Hydrant in my project. There are two problems I don't know how to solve:

Problem 1:

If there is a data struct like a tree, how to define the mapper?

eg:


@interface MyFileMeta : NSObject
@property(nonatomic, copy) NSString *basePath;
@property(nonatomic, copy) NSString *name;
@property(nonatomic, copy, nullable) NSArray< MyFileMeta  *> *children;
@end

how to define a mapper that can support uncertain depth of path tree?


Problem 2:

How to support if else branch in a mapper?

eg:

typedef NS_ENUM(NSInteger, MyProductType) {
    MyProductTypeVedio = 1,
    MyProductTypeImage,
};

@interface MyProduct : NSObject
@property(nonatomic,  copy) NSString *name;
@property(nonatomic)            double      price;
@property(nonatomic)            MyProductType type;
// ...
@property(nonatomic)            MyProductAttributes *attributes;
@end

@interface MyProductAttributes : NSObject
@property(nonatomic, copy)  NSString   *author;
// ...
@end

@interface MyVedioAttributes :  MyProductAttributes
@property(nonatomic)              NSTimeInterval   duration;
//...
@end

@interface MyImageAttributes: MyProductAttributes
@property(nonatomic)              long long fileSize;
@property(nonatomic)              long long width;
@property(nonatomic)              long long height;
// ...
@end

and here is a JSON:

[
    {"name": "move 1", "price": 111.11, "type":1,  "attributes": {"duration": "2:00:00"}}, 
    {"name": "pic1", "price": "200.00", "type":2,  "attributes": {
        "size": "10.0MB", "width": 1024, "height": 768
    }}
]

the type of "attribute" is base on the value of "type", so how to define the mapper?

Thanks a lot!

alexlee002 commented 6 years ago

Maybe it can be solve via HYDMapReflectively? I'm not sure, I try it tomorrow.