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

Revert avoidStruts Caching #118

Closed
pjones opened this issue Nov 14, 2016 · 8 comments
Closed

Revert avoidStruts Caching #118

pjones opened this issue Nov 14, 2016 · 8 comments

Comments

@pjones
Copy link
Contributor

pjones commented Nov 14, 2016

When we released v0.12 it included a bug caused by the new avoidStruts caching code. This is still a problem even though several commits were made in PR #30. (One example: creating a new dynamic workspace and then opening a window will obscure any task bars.)

@geekosaur has suggested that we pull all code related to caching into a separate branch and then revert all these changes out of master. (He also recommends the same for the _NET_WORKAREA changes.)

This issue is for us to discuss a solution before releasing v0.13. Please let us know what you think and if you'd be willing to help.

/cc @byorgey @aavogt @f1u77y

@pjones pjones added this to the v0.13 milestone Nov 14, 2016
@thomasf
Copy link

thomasf commented Nov 15, 2016

As a user I prefer to deal with workarounds rather than losing the cache
performance gains.

Inserting a few docksStartupHook calls in my config resolved all the issues I
had with the cache. I have also removed the setWorkarea in AvoidStruts (in my
own xm-contrib fork) to resolve my multi display qt/chrome menu issues. From
what I understand this causes issues with some applications but doesnt seem to
affect anything for me..

Even if I prefer to muck around a bit to get things working as I want to it's
probably not a default. If it's possible it would maybe be better to include
the cache stuff as an optional thing with a disclaimer about issues.

@geekosaur
Copy link
Contributor

I'm actually thinking along the lines of a different fix that would let us
use something like the older code, but get an even greater speed-up.
Specifically, we would cache window properties and update the cache in the
handleEventHook, avoidStruts would be able to operate via that cache
without making X11 calls itself or needing to enumerate windows; just scan
the cache for the strut properties. This would also provide noticeable
speedups for manageHook and likely at least some EWMH behaviors. The
biggest problem is that any code that looks up properties directly using
X11 bindings would need to be rewritten to use the cache; but in most cases
this would be a net simplification.

On Mon, Nov 14, 2016 at 7:29 PM, Thomas Frössman notifications@github.com
wrote:

As a user I prefer to deal with workarounds rather than losing the cache
performance gains.

Inserting a few dockssStartupHook calls in my config resolved all the
issues I
had with the cache. I have also removed the setWorkarea in AvoidStruts to
resolve my multi display qt/chrome menu issues. From what I understand this
causes issues with some applications but doesnt seem to affect anything for
me..

Even if I prefer to muck around a bit to get things working as I want to
it's
probably not a default. If it's possible it would maybe be better to
include
the cache stuff as an optional thing with it's own disclaimer about issues.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#118 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AB8SoAsSd_6V_JZYwLbCKLqDksnwfkedks5q-Pz-gaJpZM4Kx2a0
.

brandon s allbery kf8nh sine nomine associates
allbery.b@gmail.com ballbery@sinenomine.net
unix, openafs, kerberos, infrastructure, xmonad http://sinenomine.net

@f1u77y
Copy link
Contributor

f1u77y commented Nov 15, 2016

(One example: creating a new dynamic workspace and then opening a window will obscure any task bars.)

I've just tried doing that, and my status bar did not get obscured. But that behaviour could take place when using xmonad-contrib without #80(messages for cache updating are not sent to the newly created workspace).

(also agreed with @geekosaur about caching all window properties)

@pjones
Copy link
Contributor Author

pjones commented Nov 15, 2016

@f1u77y: I created an example configuration using ManageDocks and it doesn't seem to have any issues.

It did help me see that XMonad.Config.Desktop uses avoidStruts but not docksEventHook or manageDocks. We need to fix that.

My configuration still has an issue where taffybar is obscured on new workspaces. I'm going to have to try to figure it out. (BTW, I'm on master so I have #80).

@pjones
Copy link
Contributor Author

pjones commented Nov 15, 2016

Never mind about X.C.Desktop, I see that it already uses the docks function.

@colonelpanic8
Copy link
Contributor

@pjones Have you made any progress on figuring out taffybar issue. I've been seeing the same thing for a while now and was thinking about trying to look in to it.

@pjones
Copy link
Contributor Author

pjones commented Dec 1, 2016

@IvanMalison I tracked down the issue I was having to a bug in DynamicProjects which I fixed in commit 217abc3.

Based on my testing with xmonad-testing and locally with my build, I don't think avoidStruts is an issue anymore. As such, unless I hear otherwise, I'm going to close this issue.

@colonelpanic8
Copy link
Contributor

colonelpanic8 commented Dec 2, 2016

@pjones I must have a different issue then, because I am running xmonad-contrib with that commit and it is still occurring for me.

I should clarify that the issue I'm seeing actually occurs when I recompile my config/restart xmonad.

Ah, Looks like I simply wasn't applying the docks startup hook -- nevermind.

@pjones pjones closed this as completed Dec 8, 2016
liskin added a commit to liskin/xmonad-contrib that referenced this issue Nov 17, 2020
This makes docksStartupHook unnecessary. That is desirable because we
didn't add a changelog entry about it being necessary, and 4 years after
its introduction we still have users grabbing old configs and reporting
(xmonad/xmonad#21 (comment)) that
ManageDocks doesn't work properly.

I would love to finally settle this.

Full story follows:

xmonad-contrib 0.12 introduced (00be056, merged in April 2013)
caching to ManageDocks to avoid queryTree calls on every runLayout,
which brought in several bugs. Attempts to fix these bugs in
xmonad-contrib 0.13 introduced (28e9f8b, merged in February 2016) a
breaking change in ManageDocks that required users to add
docksStartupHook (or docks, after e38fb3b got merged in November
2016) to their configs for avoidStruts to still work after xmonad
restart. Unfortunately this was never mentioned in the changelog nor in
any compilation warning (which get discarded by xmonad --recompile
anyway !!!), so as of March 2020 we still have users being bitten by
this.

Back in September 2016 in a discussion about other bugs introduced in
28e9f8b I suggested that we use Maybe to indicate whether the cache
had been initialized and initialize it on demand when it had not.
Unfortunately I wasn't sufficiently clear about what I meant and Brandon
was going through some health issues, so we just got into a heated
argument and ended up merging a suboptimal solution. :-(

And since we're _still_ getting complaints from users every now and
then, I believe it's worth dealing with even after all these years.
If nothing else, let this serve as a reminder that breaking users'
configs without any warning is wrong.

(Oh and we should probably stop hiding xmonad.hs compilation warnings,
otherwise we can't ever hope to provide smooth deprecation and upgrade
paths.)

Fixes: 00be056 ("Cache results from calcGap in ManageDocks")
Fixes: 28e9f8b ("add docksStartupHook for handling docks when restarted")
Fixes: e38fb3b ("Make usage of ManageDocks simpler and more robust")
Related: xmonad#118
Related: xmonad#30
Related: xmonad#80
Related: xmonad/xmonad#21
liskin added a commit to liskin/xmonad-contrib that referenced this issue Nov 17, 2020
This makes docksStartupHook unnecessary. That is desirable because we
didn't add a changelog entry about it being necessary, and 4 years after
its introduction we still have users grabbing old configs and reporting
(xmonad/xmonad#21 (comment)) that
ManageDocks doesn't work properly.

I would love to finally settle this.

Full story follows:

xmonad-contrib 0.12 introduced (00be056, merged in April 2013)
caching to ManageDocks to avoid queryTree calls on every runLayout,
which brought in several bugs. Attempts to fix these bugs in
xmonad-contrib 0.13 introduced (28e9f8b, merged in February 2016) a
breaking change in ManageDocks that required users to add
docksStartupHook (or docks, after e38fb3b got merged in November
2016) to their configs for avoidStruts to still work after xmonad
restart. Unfortunately this was never mentioned in the changelog nor in
any compilation warning (which get discarded by xmonad --recompile
anyway !!!), so as of March 2020 we still have users being bitten by
this.

Back in September 2016 in a discussion about other bugs introduced in
28e9f8b I suggested that we use Maybe to indicate whether the cache
had been initialized and initialize it on demand when it had not.
Unfortunately I wasn't sufficiently clear about what I meant and Brandon
was going through some health issues, so we just got into a heated
argument and ended up merging a suboptimal solution. :-(

And since we're _still_ getting complaints from users every now and
then, I believe it's worth dealing with even after all these years.
If nothing else, let this serve as a reminder that breaking users'
configs without any warning is wrong.

(Oh and we should probably stop hiding xmonad.hs compilation warnings,
otherwise we can't ever hope to provide smooth deprecation and upgrade
paths.)

Fixes: 00be056 ("Cache results from calcGap in ManageDocks")
Fixes: 28e9f8b ("add docksStartupHook for handling docks when restarted")
Fixes: e38fb3b ("Make usage of ManageDocks simpler and more robust")
Related: xmonad#118
Related: xmonad#30
Related: xmonad#80
Related: xmonad/xmonad#21
liskin added a commit to liskin/xmonad-contrib that referenced this issue Nov 17, 2020
This makes docksStartupHook unnecessary. That is desirable because we
didn't add a changelog entry about it being necessary, and 4 years after
its introduction we still have users grabbing old configs and reporting
(xmonad/xmonad#21 (comment)) that
ManageDocks doesn't work properly.

I would love to finally settle this.

Full story follows:

xmonad-contrib 0.12 introduced (00be056, merged in April 2013)
caching to ManageDocks to avoid queryTree calls on every runLayout,
which brought in several bugs. Attempts to fix these bugs in
xmonad-contrib 0.13 introduced (28e9f8b, merged in February 2016) a
breaking change in ManageDocks that required users to add
docksStartupHook (or docks, after e38fb3b got merged in November
2016) to their configs for avoidStruts to still work after xmonad
restart. Unfortunately this was never mentioned in the changelog nor in
any compilation warning (which get discarded by xmonad --recompile
anyway !!!), so as of March 2020 we still have users being bitten by
this.

Back in September 2016 in a discussion about other bugs introduced in
28e9f8b I suggested that we use Maybe to indicate whether the cache
had been initialized and initialize it on demand when it had not.
Unfortunately I wasn't sufficiently clear about what I meant and Brandon
was going through some health issues, so we just got into a heated
argument and ended up merging a suboptimal solution. :-(

And since we're _still_ getting complaints from users every now and
then, I believe it's worth dealing with even after all these years.
If nothing else, let this serve as a reminder that breaking users'
configs without any warning is wrong.

(Oh and we should probably stop hiding xmonad.hs compilation warnings,
otherwise we can't ever hope to provide smooth deprecation and upgrade
paths.)

Fixes: 00be056 ("Cache results from calcGap in ManageDocks")
Fixes: 28e9f8b ("add docksStartupHook for handling docks when restarted")
Fixes: e38fb3b ("Make usage of ManageDocks simpler and more robust")
Related: xmonad#118
Related: xmonad#30
Related: xmonad#80
Related: xmonad/xmonad#21
AusCyberman pushed a commit to AusCyberman/xmonad-contrib that referenced this issue Jan 25, 2021
This makes docksStartupHook unnecessary. That is desirable because we
didn't add a changelog entry about it being necessary, and 4 years after
its introduction we still have users grabbing old configs and reporting
(xmonad/xmonad#21 (comment)) that
ManageDocks doesn't work properly.

I would love to finally settle this.

Full story follows:

xmonad-contrib 0.12 introduced (00be056, merged in April 2013)
caching to ManageDocks to avoid queryTree calls on every runLayout,
which brought in several bugs. Attempts to fix these bugs in
xmonad-contrib 0.13 introduced (28e9f8b, merged in February 2016) a
breaking change in ManageDocks that required users to add
docksStartupHook (or docks, after e38fb3b got merged in November
2016) to their configs for avoidStruts to still work after xmonad
restart. Unfortunately this was never mentioned in the changelog nor in
any compilation warning (which get discarded by xmonad --recompile
anyway !!!), so as of March 2020 we still have users being bitten by
this.

Back in September 2016 in a discussion about other bugs introduced in
28e9f8b I suggested that we use Maybe to indicate whether the cache
had been initialized and initialize it on demand when it had not.
Unfortunately I wasn't sufficiently clear about what I meant and Brandon
was going through some health issues, so we just got into a heated
argument and ended up merging a suboptimal solution. :-(

And since we're _still_ getting complaints from users every now and
then, I believe it's worth dealing with even after all these years.
If nothing else, let this serve as a reminder that breaking users'
configs without any warning is wrong.

(Oh and we should probably stop hiding xmonad.hs compilation warnings,
otherwise we can't ever hope to provide smooth deprecation and upgrade
paths.)

Fixes: 00be056 ("Cache results from calcGap in ManageDocks")
Fixes: 28e9f8b ("add docksStartupHook for handling docks when restarted")
Fixes: e38fb3b ("Make usage of ManageDocks simpler and more robust")
Related: xmonad#118
Related: xmonad#30
Related: xmonad#80
Related: xmonad/xmonad#21
AusCyberman pushed a commit to AusCyberman/xmonad-contrib that referenced this issue Jan 25, 2021
This makes docksStartupHook unnecessary. That is desirable because we
didn't add a changelog entry about it being necessary, and 4 years after
its introduction we still have users grabbing old configs and reporting
(xmonad/xmonad#21 (comment)) that
ManageDocks doesn't work properly.

I would love to finally settle this.

Full story follows:

xmonad-contrib 0.12 introduced (00be056, merged in April 2013)
caching to ManageDocks to avoid queryTree calls on every runLayout,
which brought in several bugs. Attempts to fix these bugs in
xmonad-contrib 0.13 introduced (28e9f8b, merged in February 2016) a
breaking change in ManageDocks that required users to add
docksStartupHook (or docks, after e38fb3b got merged in November
2016) to their configs for avoidStruts to still work after xmonad
restart. Unfortunately this was never mentioned in the changelog nor in
any compilation warning (which get discarded by xmonad --recompile
anyway !!!), so as of March 2020 we still have users being bitten by
this.

Back in September 2016 in a discussion about other bugs introduced in
28e9f8b I suggested that we use Maybe to indicate whether the cache
had been initialized and initialize it on demand when it had not.
Unfortunately I wasn't sufficiently clear about what I meant and Brandon
was going through some health issues, so we just got into a heated
argument and ended up merging a suboptimal solution. :-(

And since we're _still_ getting complaints from users every now and
then, I believe it's worth dealing with even after all these years.
If nothing else, let this serve as a reminder that breaking users'
configs without any warning is wrong.

(Oh and we should probably stop hiding xmonad.hs compilation warnings,
otherwise we can't ever hope to provide smooth deprecation and upgrade
paths.)

Fixes: 00be056 ("Cache results from calcGap in ManageDocks")
Fixes: 28e9f8b ("add docksStartupHook for handling docks when restarted")
Fixes: e38fb3b ("Make usage of ManageDocks simpler and more robust")
Related: xmonad#118
Related: xmonad#30
Related: xmonad#80
Related: xmonad/xmonad#21
AusCyberman pushed a commit to AusCyberman/xmonad-contrib that referenced this issue Feb 2, 2021
This makes docksStartupHook unnecessary. That is desirable because we
didn't add a changelog entry about it being necessary, and 4 years after
its introduction we still have users grabbing old configs and reporting
(xmonad/xmonad#21 (comment)) that
ManageDocks doesn't work properly.

I would love to finally settle this.

Full story follows:

xmonad-contrib 0.12 introduced (00be056, merged in April 2013)
caching to ManageDocks to avoid queryTree calls on every runLayout,
which brought in several bugs. Attempts to fix these bugs in
xmonad-contrib 0.13 introduced (28e9f8b, merged in February 2016) a
breaking change in ManageDocks that required users to add
docksStartupHook (or docks, after e38fb3b got merged in November
2016) to their configs for avoidStruts to still work after xmonad
restart. Unfortunately this was never mentioned in the changelog nor in
any compilation warning (which get discarded by xmonad --recompile
anyway !!!), so as of March 2020 we still have users being bitten by
this.

Back in September 2016 in a discussion about other bugs introduced in
28e9f8b I suggested that we use Maybe to indicate whether the cache
had been initialized and initialize it on demand when it had not.
Unfortunately I wasn't sufficiently clear about what I meant and Brandon
was going through some health issues, so we just got into a heated
argument and ended up merging a suboptimal solution. :-(

And since we're _still_ getting complaints from users every now and
then, I believe it's worth dealing with even after all these years.
If nothing else, let this serve as a reminder that breaking users'
configs without any warning is wrong.

(Oh and we should probably stop hiding xmonad.hs compilation warnings,
otherwise we can't ever hope to provide smooth deprecation and upgrade
paths.)

Fixes: 00be056 ("Cache results from calcGap in ManageDocks")
Fixes: 28e9f8b ("add docksStartupHook for handling docks when restarted")
Fixes: e38fb3b ("Make usage of ManageDocks simpler and more robust")
Related: xmonad#118
Related: xmonad#30
Related: xmonad#80
Related: xmonad/xmonad#21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants