eric-bixby / auto-sort-bookmarks-webext

Firefox add-on to sort bookmarks by multiple criteria
GNU General Public License v3.0
112 stars 21 forks source link

Firefox hanging #29

Closed aspasch closed 8 years ago

aspasch commented 8 years ago

Firefox (45.2.0) hanging just about few minuts of use. Turn it off and Firefox back to work.

antoyo commented 8 years ago

There is currently no maintainer for this add-on. I'm sorry, but if no one gets involved to support this add-on, it will soon (if it is not already the case) stop working.

eric-bixby commented 8 years ago

I keep the auto feature turned off. Instead click on the add-on icon to manually sort. Not as convenient but good enough.

On Mon, Jul 18, 2016 at 9:51 AM aspasch notifications@github.com wrote:

Firefox (45.2.0) hanging just about few minuts of use. Turn it off and Firefox back to work.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/antoyo/auto-sort-bookmarks/issues/29, or mute the thread https://github.com/notifications/unsubscribe-auth/AE0RMU8sdl4UkmKL2mUYGG1AsiCelinbks5qW68pgaJpZM4JO6ml .

Gitoffthelawn commented 8 years ago

@antoyo Have you posted a "request for maintainer" on the AMO forum? Maybe someone will pick it up.

eric-bixby commented 8 years ago

This issue is most likely a dup of issue #19.

I've seen a few people offer to take over, but I've seen zero contributions since @antoyo announced a "request for maintainer" last year. Did they give up?

antoyo commented 8 years ago

@Gitoffthelawn Yes and no one answered. I've also posted on some other forums without much success. Moreover, one update of the add-on show a message to ask for a maintener.

@TheRealMarnes did some work on this add-on for some time. This is appreciated.

eric-bixby commented 8 years ago

@antoyo. I'd hate to see this add-on go away. I was going to offer before, but I thought you had someone. I'm primarily a Java developer, but I have experience doing JavaScript also.

I realize everyone is different, but as a frame of reference, how much time do you allocate yourself on a daily/weekly basis for being the maintainer?

leaumar commented 8 years ago

@antoyo thanks for the mention but tbh I don't recall doing anything noteworthy on it... I gave it up pretty quickly. Sorry.

antoyo commented 8 years ago

@eric-bixby If you wish to only update the add-on so that it stays compatible with the new firefox releases, you will probably need a couple of hours per month (plus an initial time investment at the beginning to understand all the code and fix the issues like this one).

But if you wish to add new features and such, that will more likely be a few hours per week… Or even more since I think a complete rewrite of the add-on would be beneficial, for instance to take advantage of e10s when it is good enough to sort the bookmarks concurrently (currently, the sorting "waits" from time to time so that it does not make Firefox freeze — though, apparently, it now does).

Thanks for your interest.

Gitoffthelawn commented 8 years ago

@eric-bixby Although concurrent sorting would be beneficial, it's not a requirement, so I wouldn't let that get in your way.

Also, if you wanted to simplify things, you could have it work in manual mode only.

Basically, a working extension with fewer features is better than a non-working extension with all the bells and whistles.

eric-bixby commented 8 years ago

The manual sort works fine, so the freeze is most likely caused by a race-condition in the Thread class. As @antoyo says, it's supposed to wait so as to not lockup Firefox, but apparently that's broken with the newer Firefox.

I agree disabling the auto-sort feature for now makes sense until a more stable solution can be worked out. Hopefully, people don't complain too much that the Auto-sort is not auto.

On Mon, Jul 18, 2016 at 9:13 PM, Gitoffthelawn notifications@github.com wrote:

@eric-bixby https://github.com/eric-bixby Although concurrent sorting would be beneficial, it's not a requirement, so I wouldn't let that get in your way.

Also, if you wanted to simplify things, you could have it work in manual mode only.

Basically, a working extension with fewer features is better than a non-working extension with all the bells and whistles.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/antoyo/auto-sort-bookmarks/issues/29#issuecomment-233525674, or mute the thread https://github.com/notifications/unsubscribe-auth/AE0RMcuQlKmgQOk8uo4Mkh970jmuhMmkks5qXE7agaJpZM4JO6ml .

Gitoffthelawn commented 8 years ago

@eric-bixby If you choose to maintain the extension (or fork it), and remove the auto-sort, I think renaming it would be reasonable, so that such complaints could be avoided.

antoyo commented 8 years ago

To fix this issue, someone could check, first of all, if there are errors in the error console. Then, we could try setting a timeout with a value more than 0 here to see if this fixes the issue (I think it was 100 in a previous version of the add-on). It will make the sorting slower, though. Another idea would be to check if the bookmark observer behaves correctly (because I think it is the only code that runs in auto-sort and not in manual sort). If nothing obvious is found out by these checks/updates, then we could easily disable the auto-sort option.

eric-bixby commented 8 years ago

@Gitoffthelawn: I'd prefer to keep the product name as-is (for product recognition) and just send complaints to /dev/null. Hopefully, disabling auto-sort will be temporary. Also, the manual-sort not working, might just be in FF 45.X. I noticed the icon looks disabled (possible color choice), regardless if that's the issue or not, I'd like to update the icon color and also have it change while a sort is in progress.

@antoyo: I'm planning to do my thread analysis this weekend when I have more time. BTW, I'm doing my work in the following fork: https://github.com/eric-bixby/auto-sort-bookmarks

antoyo commented 8 years ago

@eric-bixby Do you wish to be added as a contributor on this GitHub project?

Also, I will need to add you as an author on AMO so that you can upload a new version. To do so, you will need to send me the email address you used to register on AMO (you can send it to me by email if do you not wish to make it public).

Thanks for your work.

antoyo commented 8 years ago

@eric-bixby I added you as a collaborator on GitHub and as an author on AMO. I am not sure it worked on AMO though. Could you please tell me if you received an invitation or something?

eric-bixby commented 8 years ago

GitHub worked. However, I didn't receive any email for AMO.and it doesn't list me on the website. On Thu, Jul 21, 2016 at 7:21 AM antoyo notifications@github.com wrote:

@eric-bixby https://github.com/eric-bixby I added you as a collaborator on GitHub and as an author on AMO. I am not sure it worked on AMO though. Could you please tell me if you received an invitation or something?

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/antoyo/auto-sort-bookmarks/issues/29#issuecomment-234268559, or mute the thread https://github.com/notifications/unsubscribe-auth/AE0RMWZu8sHTJbG2ViavYAHtVTzJOss5ks5qX4BWgaJpZM4JO6ml .

antoyo commented 8 years ago

@eric-bixby I submitted a bug report on the AMO forum. I hope it will be fixed soon.

eric-bixby commented 8 years ago

Yes, it works now. Thanks On Thu, Jul 21, 2016 at 9:32 AM antoyo notifications@github.com wrote:

@eric-bixby https://github.com/eric-bixby I submitted a bug report on the AMO forum. I hope it will be fixed soon.

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/antoyo/auto-sort-bookmarks/issues/29#issuecomment-234309358, or mute the thread https://github.com/notifications/unsubscribe-auth/AE0RMQsf41wIHEuyDv0o0jJEwrGz3N20ks5qX58CgaJpZM4JO6ml .

antoyo commented 8 years ago

One solution could be to revert back to version 2.7 (as it seems to be less CPU intensive) and apply only the fixes of the subsequent commits (not the new features) so that it works with the latest Firefox version (the add-on will lose some features though).

If you have any question, feel free to ask.

eric-bixby commented 8 years ago

I tried the 2.7 version (just had to tweak the package.json to compile it). It doesn't work as-is with the latest FF, so as you said, it would require patching. However, that could be just as much work or more work than fixing the current code. I feel confident that the problem with the current code is the threading. I'm currently trying to figure out how to use the Firefox Profiler (lockups make this difficult).

antoyo commented 8 years ago

Well, I'll let you manage this the way you want, but the version 2.7 was already fixed by the subsequent commits and this version used the Thread class as well, so this class may be good (or perhaps a new version of Firefox broke it, I don't know). Anyway, thanks for your work.

Gitoffthelawn commented 8 years ago

@eric-bixby Thanks for everything you are doing for this project!

eric-bixby commented 8 years ago

My preliminary investigation is leaning toward an issue with the version of Firefox used. I was not able to reproduce the high-load/freezing under FF 47.0.1. While I did see it using FF 46.0.1. I need to do more testing to determine which versions the issue is and isn't seen in. Also, I'd like to know what the root cause is and find a workaround (other than upgrading Firefox).

eric-bixby commented 8 years ago

This is a duplicate of issue #19.

Please see that issue for status updates.