yojimbo87 / ArangoDB-NET

C#/.NET/Mono driver for ArangoDB
MIT License
107 stars 66 forks source link

New method: ToListFast<T> uses fastJSON to deserialize much quicker #28

Closed specimen151 closed 8 years ago

specimen151 commented 8 years ago

Hi. I noticed that the current AQuery.ToList<T> is very slow. So I made this new method instead, which use the fastJSON already used in the project. When profiling with Visual Studio, for my example collection (400 lines pretty print JSON) it was up to 40x quicker.

The best solution would of course be to rewrite the AQuery.ToList<T> instead of creating a new method, but I leave that up to you, because I haven't had the time to really grok you code, so maybe my ToListFast has a hug bug I don't know about...

specimen151 commented 8 years ago

I created a second commit, in which the original ToObject<T> is much quicker (but still only half as quick as ToObjectFast). What I did was to remove Exception's as the means to report missing fields. Visual Studio Profiler reported "# of Exceps Thrown / Sec high". It is much quicker without all the exceptions, and my personal opinion is that when checking if a field exists, it is not an exceptional event if it doesn't exist, more like "yes/no". But that's my opinion of course.

yojimbo87 commented 8 years ago

Thanks a lot! I will look into it during the weekend.

yojimbo87 commented 8 years ago

In the case of HasFieldValue method which doesn't throw exceptions - if I remember correctly those were added there to distinguish between various states, one of them being if you are requesting some specific field which is not present in the dictionary, which in my opinion is a case for exception (for example same as the case when you looking for file which doesn't exist and you get an exception).

Of course if you are just checking fields for their existence then this might not be the most optimal way of doing things, however GetFieldValue was reused also for other purpose like retrieving values or type checking where true/false obviously wasn't enough. I will have to look into it if both methods could be refactored into one so that there are not two very similar operations.

specimen151 commented 8 years ago

I agree that it is subjective when a situation is "exceptional", and you have a good point, maybe better than mine. The challenge is that exceptions are slow (when they come in large numbers). So are properties by the way (compared to public member variables), of course only when talking about large numbers (like creating a list of objects from a reasonably sized JSON file, and then having many users doing this).

I really like your project, so I'm glad you're actively developing and improving it all the time.

yojimbo87 commented 8 years ago

I published new version of the driver which is now using fastJSON ToObject deserialization in most cases. Can you please profile current version in VS and let me know if the performance is better for you?

specimen151 commented 8 years ago

I'll try to do it this week, and let you know.

specimen151 commented 8 years ago

Hi. I copied my ToListFast<T>() into the repo temporarily to do a comparison, and can confirm that your new version (of ToList<T>()) is slightly faster. So now I'm back with the official branch.

yojimbo87 commented 8 years ago

Ok, thanks for letting me know.