firescript / nativescript-contacts

A nativescript module that gives access to the native contact directory.
MIT License
28 stars 32 forks source link

Performance is slow #13

Closed ryc16 closed 7 years ago

ryc16 commented 8 years ago

I try on the real device galaxy note 2. It take 17s to get 100 contact records. I thought and expect to be quick but it is not. Any idea why it is extremely slow. Implementing directly from Android take less than 1 second to get 100 contact.

What is the problem and any advice how to improve the speed to get the device contacts.

harlankoehn commented 8 years ago

This is a very big deal to me too, though I haven't yet tested it to confirm ryc16's claims. Any comments from the developers on this? Is this caused by the fact that NativeScript uses a Javascript Virtual Machine or is there some unoptimized code in NativeScript somewhere?

PeterStaev commented 8 years ago

Hey @ryc16 , how much info do you load when you implement contact loading directly in Android? Currently the plugin always loads all the data for the contact - names, photo, all phones, addresses, emails, links, notes, so i suspect this is why it is slower. But since i don't have an actual android device i cannot be sure.

sitefinitysteve commented 8 years ago

Wonder if the best way to go about it is to change getAllContacts to allow the user to optionally pass in the data they need. So like could just be first\last\avatar, then the extra cursors wouldn't be necessary... full data could be obtained by getting the single contact itself (as always, no change).

The iOS side kinda already has this, they are hardcoded though Link Could standardize and impliment on both sides.

PeterStaev commented 8 years ago

I agree @sitefinitysteve . For iOS I think we will also have to adjust the helper function here so that it puts undefined if the key is not available.

abhayastudios commented 8 years ago

I have something similar on a Samsung Galaxy S4 where it takes ~5mins to load ~1000 contacts (includes Whatsapp and Exchange contacts). I am only interested in the names and phone numbers (so no need to get all data for every contact).

sitefinitysteve commented 8 years ago

I promise we'll get to it, just busy atm :/ issue is not {N} related but rather how were converting all the data... Lots of cursors, no perf problems on the simulators or devices with not many contacts, sorry!

abhayastudios commented 8 years ago

@sitefinitysteve thanks for responding to this. Obviously I cannot manage your priorities, but I disagree that this is not a {N} issue. The issue for {N} is that things like these prevent more adoption. For example I am currently evaluating whether {N} in combination with the Telerik backend is the right tool for a job. Although I really like the concept so far, there are lots of issues not only with this plugin but with the wider ecosystem (for example I am also struggling to get the Everlive SDK to work with v2). So I will definitely keep following {N} although I think that for the job at hand I will choose another platform.

If you are an independent developer then I can only say thanks for putting this out there and I hope for the community that you will get to it at some point (because it currently is not workable on a real Android device). If you are on the Telerik payroll then I would politely ask you to reconsider :)

sitefinitysteve commented 8 years ago

@abhayastudios We do not work for telerik... We needed this functionality in nativescript so we made the plugin. The speed issue is not a NativeScript issue is what i meant, it's not a framework limitation... Its our implementation.

ryc16 commented 8 years ago

Hi all, I like the {N} and the concept with pros and cons. However, I feel {N} is still lack of examples, and resources. It is a bit hard to find information from internet. Market/technology is growing/moving very fast. I hope {N} could grow bigger and faster to provide as many features as possible out of box not to behind market. One concept form {N} to allow direct access native api seems good, but on the other hand, if we often need to access native api, we are pretty much coding in native for different platform which is not the purpose of choosing {N}. I am still struggling to use {N} as I got more issues but can't find solution and not sure if issue can be resolved early enough.

@abhayastudios, understood you are not working for telerik and you are busy on other work, However, accessing the phone contact is very common or basic things. It really the key thing and block me. I wish you could take a time to have a look as you mentioned it is your implementation not because of {N} framework.

(Note: if you have many contacts even on emulator, you will face the same performance issue)

Let me have some feedback. I kind of agree that the slowness might come from the cursor. @PeterStaev, However, I don't agree limiting to field is the solution as people need all those fields. So I think it is important to resolve the bottleneck on the cursor.

I am appreciate if anyone can look into it as contact is really a basic thing. Any advice for workaround or bypass the problem with sample would be nice.

PeterStaev commented 8 years ago

Hey @ryc16 , my wild guess is that if you return ALL the data we are returning now (and from what I see this can be done ONLY with cursors) in native Android you will face the same problem. So once again this is a bottleneck of the native platform not with {N} (or the implementation we have). So the only way to speed things up is to limit the amount of data that is returned (what Corodva does in their plugin). We can only do as much as the specific native platform allows us to do.

If you want to prove me wrong - try to return ALL the fields we are returning currently coding it in native Java and see if you will get much better performance.

bnussey commented 8 years ago

Any updates on the ability to optionally pass in what fields are needed? I am also experiencing slow performance on this.

sitefinitysteve commented 8 years ago

Suuuper busy with a million projects, have not forgotten though! Soon, very soon

lawrencetaur commented 7 years ago

Would something like web-workers speed up the processing if this is on a another thread?

abhayastudios commented 7 years ago

Anyone here in for pooling together some money to have a more efficient plugin developed (or improve this one) and open source it?

bnussey commented 7 years ago

Hey @abhayastudios we would be, let me know how you want to get going.

abhayastudios commented 7 years ago

@bnussey I asked someone from the NS community to provide me with a cost estimate, so I'll update once I have more. In the meantime if anyone else wants to pitch in that would be great :)

bnussey commented 7 years ago

Great you can hit me up on nativescript community slack, username - blake

abhayastudios commented 7 years ago

@sitefinitysteve I am playing around with a clone of the plugin. By simply passing an array of desired fields and then bypassing the irrelevant cursors, I was able to cut down the processing time from ~5 mins to ~35s on an old Android with ~600 contacts when requesting the name, phoneNumbers and photo fields (which is good enough for my app).

This is obviously much better, but still not good enough. I will continue playing around with it in the next couple of days, I was just wondering whether you are aware of any other low hanging fruit that I might attend to.

sitefinitysteve commented 7 years ago

It needs to be reworked to use the new webworker feature in 2.4 as well.

Just no time....

On Dec 14, 2016 6:21 AM, "Abhaya Studios" notifications@github.com wrote:

@sitefinitysteve https://github.com/sitefinitysteve I am playing around with a clone of the plugin. By simply passing an array of desired fields and then bypassing the irrelevant cursors, I was able to cut down the processing time from ~5 mins to ~35s on an old Android with ~600 contacts when requesting the name, phoneNumbers and photo fields (which is good enough for my app).

This is obviously much better, but still not good enough. I will continue playing around with it in the next couple of days, I was just wondering whether you are aware of any other low hanging fruit that I might attend to.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/firescript/nativescript-contacts/issues/13#issuecomment-267009886, or mute the thread https://github.com/notifications/unsubscribe-auth/ABeI6P-y3vebBXVaCK-4pqEBoN4gZOk1ks5rH9EsgaJpZM4I-nTQ .

abhayastudios commented 7 years ago

@sitefinitysteve I can give it a try, but as far as I know web workers (at least on browsers) do not really speed up a process in this case they just offload the UI thread but it is still running in a single thread or am I missing something. I was more thinking along the lines of more efficient ways to retrieve data from the Android/iOS source tables, or maybe some other processing/conversion.

I will have a look at the cordova contacts plugin as well to see how they do it. It is pretty fast... Anyway if you think of anything else please let me know.

lawrencetaur commented 7 years ago

@NathanaelA said he has tried a web workers implementation which scanned 2000 contacts pretty fast. Not sure if he is open to sharing it though he did have plans to.

abhayastudios commented 7 years ago

@lawrencetaur I spoke to him but he said he is not able to open source that project. I am sure though he had a different approach as web workers are great but not a silver bullet. I'll give it a try anyway, because I see that while it is running the UI pretty much freezes, which is annoying even if we are able to trim the processing down to a couple of seconds.

sitefinitysteve commented 7 years ago

Nathanael created a variant of this using workers and it completes near instantly. It does offload off the ui thread, but that's what we want anyway.

On Dec 14, 2016 8:56 AM, "Abhaya Studios" notifications@github.com wrote:

@sitefinitysteve https://github.com/sitefinitysteve I can give it a try, but as far as I know web workers (at least on browsers) do not really speed up a process in this case they just offload the UI thread but it is still running in a single thread or am I missing something. I was more thinking along the lines of more efficient ways to retrieve data from the Android/iOS source tables, or maybe some other processing/conversion.

I will have a look at the cordova contacts plugin as well to see how they do it. It is pretty fast... Anyway if you think of anything else please let me know.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/firescript/nativescript-contacts/issues/13#issuecomment-267039571, or mute the thread https://github.com/notifications/unsubscribe-auth/ABeI6MTbI-cfqCbp9jVw-Fu3O8ktfKyJks5rH_WOgaJpZM4I-nTQ .

abhayastudios commented 7 years ago

@sitefinitysteve so you are saying just moving to a web worker improves the performance without making changes to the project itself? Wow sounds too good to be true :) I'll give it a try and update you guys.

sitefinitysteve commented 7 years ago

Well I mean it's where we need to go REGARDLESS :) To not block UI... his requirement was a autocomplete userpicker

abhayastudios commented 7 years ago

The good news is that I was able to turn it into a web worker, which is cool because the UI is released immediately and keeps working smooth. The bad news is that, as I suspected, it didn't make the performance any better (actually the web worker process adds about 0.5s of overhead). I spoke with Nathanael about it he said that the problem is in the architecture. I will get some more pointers from him and try to improve it even more.

abhayastudios commented 7 years ago

I've committed my changes to this fork if anyone wants to try out. So far it includes some changes to make the plugin work inside a web worker and the functions getAllContacts & getContactsByName now take an argument contactFields where you define what data you are interested in to narrow down the result set. The usage part contains an example of how to run it.

Current status

When limiting the results set by passing contactFields = ['name','phoneNumbers']

iPhone

@bnussey ran it on an iPhone 5s with latest iOS it returned virtually instantly with ~600 contacts

Android

On my 3 year old Samsung Galaxy S4 it took about 35s to obtain about ~600 contacts. Still too slow but a significant improvement from the original 5 mins. I will have another go at improving it in the meantime.

Regression

The getContact function doesn't work inside of a web worker since it tries to attach to the top window. On iOS it actually throws all kinds of warnings about it (but it will still work).

Is anyone using this function? Maybe it can be dropped altogether. As far as I understand you can build more or less the same functionality with the other methods.

PeterStaev commented 7 years ago

For the getContact method I do not think we can remove it as it uses the built in contact pickers for easy working with selecting contacts. If we remove it then users will have to create their own contact pickers. Also I do not think this should be offloaded on a worker as it does not have any heavy work doing in it.

abhayastudios commented 7 years ago

@PeterStaev for now I went with putting the entire plugin inside a web worker, but you are right I will try to invoke a web worker only for the getAllContacts & getContactsByName functions. That way the getContact will still work.

abhayastudios commented 7 years ago

@PeterStaev I have reworked it a bit so that only the functions getAllContacts & getContactsByName run inside a web worker and that it doesn't break compatibility with getContact. I've submitted this in PR #18.

Having said that, for some reason that is not entirely clear to me yet, this way of handling it is slower than when I run the entire plugin in a web worker. This way on my 3 year old Samsung Galaxy S4 it takes about 50s to obtain about ~600 contacts (when limited query scope to 'name' and 'phoneNumbers'), whereas when I call the plugin from within a web worker it takes about 35s.

abhayastudios commented 7 years ago

I wrote another plugin almost from scratch and named it nativescript-contacts-lite. The purpose is to get pretty fast read-only access to the contacts directory. On a relatively old Samsung Galaxy S4 a list of ~600 contacts is returned somewhere between ~500ms up to ~2s depending on the desired fields. Also it runs inside of a web worker so it will make sure that the UI thread won't get stuck.

Currently it supports only Android, but I will add iOS support in a couple of days hopefully. I would be glad for someone to try it out and see if it is working properly for them.

Happy holidays :slightly_smiling_face:

firescript commented 7 years ago

@abhayastudios merged your PR, which should fix this? Sorry I totally didn't see your PR in December.