Skip to content
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.

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

Firefox hanging #29

Closed
aspasch opened this issue Jul 18, 2016 · 24 comments
Closed

Firefox hanging #29

aspasch opened this issue Jul 18, 2016 · 24 comments
Assignees
Labels
Priority: 2-High Serious problem that could block progress Status: Completed The issue is closed Type: Bug The issue documents a broken, incorrect, or confusing feature behavior

Comments

@aspasch
Copy link

aspasch commented Jul 18, 2016

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

@antoyo
Copy link
Contributor

antoyo commented Jul 18, 2016

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
Copy link
Owner

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
#29, or mute the
thread
https://github.com/notifications/unsubscribe-auth/AE0RMU8sdl4UkmKL2mUYGG1AsiCelinbks5qW68pgaJpZM4JO6ml
.

@Gitoffthelawn
Copy link
Contributor

Gitoffthelawn commented Jul 18, 2016

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

@eric-bixby
Copy link
Owner

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
Copy link
Contributor

antoyo commented Jul 18, 2016

@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
Copy link
Owner

@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
Copy link
Contributor

leaumar commented Jul 18, 2016

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

@antoyo
Copy link
Contributor

antoyo commented Jul 18, 2016

@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
Copy link
Contributor

@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
Copy link
Owner

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
#29 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AE0RMcuQlKmgQOk8uo4Mkh970jmuhMmkks5qXE7agaJpZM4JO6ml
.

@Gitoffthelawn
Copy link
Contributor

@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
Copy link
Contributor

antoyo commented Jul 19, 2016

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
Copy link
Owner

@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
Copy link
Contributor

antoyo commented Jul 20, 2016

@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
Copy link
Contributor

antoyo commented Jul 21, 2016

@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
Copy link
Owner

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
#29 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AE0RMWZu8sHTJbG2ViavYAHtVTzJOss5ks5qX4BWgaJpZM4JO6ml
.

@antoyo
Copy link
Contributor

antoyo commented Jul 21, 2016

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

@eric-bixby
Copy link
Owner

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
#29 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AE0RMQsf41wIHEuyDv0o0jJEwrGz3N20ks5qX58CgaJpZM4JO6ml
.

@antoyo
Copy link
Contributor

antoyo commented Jul 22, 2016

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
Copy link
Owner

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
Copy link
Contributor

antoyo commented Jul 23, 2016

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
Copy link
Contributor

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

@eric-bixby
Copy link
Owner

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 eric-bixby self-assigned this Aug 5, 2016
@eric-bixby eric-bixby added Type: Bug The issue documents a broken, incorrect, or confusing feature behavior duplicate labels Aug 5, 2016
@eric-bixby
Copy link
Owner

This is a duplicate of issue #19.

Please see that issue for status updates.

@eric-bixby eric-bixby added Priority: 2-High Serious problem that could block progress Status: Completed The issue is closed labels Nov 29, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority: 2-High Serious problem that could block progress Status: Completed The issue is closed Type: Bug The issue documents a broken, incorrect, or confusing feature behavior
Projects
None yet
Development

No branches or pull requests

5 participants