Liupold / pidswallow

A swallower script using process hierarchy.
Apache License 2.0
43 stars 0 forks source link

minimal way to support each and every term. #40

Closed Liupold closed 4 years ago

Liupold commented 4 years ago

Should fix daemon based terms completely.

SeerLite commented 4 years ago

I don't understand this one. Shells don't have WIDs, only the terminals, right? Or am I missing something? o_o

Edit: This does give me an idea though...

Liupold commented 4 years ago

I don't understand this one. Shells don't have WIDs, only the terminals, right? Or am I missing something? o_o

Edit: This does give me an idea though...

Yes but if you run $(xdo id) you get the WID of the terminal. Does it work.??

SeerLite commented 4 years ago

Ah I'm stupid I didn't read your changes to the README! Yeah I get what you're doing now

Liupold commented 4 years ago

This is the best solution i came up with, Let me know if this can be improved.

Liupold commented 4 years ago

BTW, we can't use -1 in xev. It's just been added in April, A lot of "stable" distro won't have that for a year or so. (I guess).

Liupold commented 4 years ago

should fix: #36 #38 #17.

Liupold commented 4 years ago

@SeerLite when you are ready feel fee to merge this.

SeerLite commented 4 years ago

Hey, just wanted to ask why you added the decimal conversion to WID values? (I can't remember)

Currently it seems to work fine with the hex ones and they look a lot nicer in the verbose output. Do you think it's ok to change them back?

SeerLite commented 4 years ago

Also I can't understand the changes in fdb921ecb8e45de947adb456fa813bf6f8ea5666. Are we running the preglue hook twice now?

Edit: Sorry if I'm a bit late with these questions, I hadn't analyzed these bits before and now I just wanna understand them :p

Liupold commented 4 years ago

Hey, just wanted to ask why you added the decimal conversion to WID values? (I can't remember)

Currently it seems to work fine with the hex ones and they look a lot nicer in the verbose output. Do you think it's ok to change them back?

different source of wid had different convention. (And it's a real pain). so decimal. it might be possible though(to use "%x"). (small capital letters and zeros, Eg: 0x0E78 and 0xe78 are same).

change printf "%d" "$wid" to prinf "%x" "$wid" it might work.

Liupold commented 4 years ago

Also I can't understand the changes in fdb921e. Are we running the preglue hook twice now?

Edit: Sorry if I'm a bit late with these questions, I hadn't analyzed these bits before and now I just wanna understand them :p

hey it's fine.

Liupold commented 4 years ago

Also I can't understand the changes in fdb921e. Are we running the preglue hook twice now?

My bad delete ta last preglue cmd. (Not needed).

SeerLite commented 4 years ago

it might be possible though(to use "%x"). (small capital letters and zeros, Eg: 0x0E78 and 0xe78 are same).

Capital is fine too, right? I went for "%X".

Liupold commented 4 years ago

if you are working on the script you might want to use regex instead of grep in pgeo.

SeerLite commented 4 years ago

Also I can't understand the changes in fdb921e. Are we running the preglue hook twice now?

My bad delete ta last preglue cmd. (Not needed).

Do you mean last as in "last in the code" or "last added"? Is there a reason for preglue happening before the swallowing?

Liupold commented 4 years ago

it might be possible though(to use "%x"). (small capital letters and zeros, Eg: 0x0E78 and 0xe78 are same).

Capital is fine too, right? I went for "%X".

Better to use "%x" (similar to xprop and xev). (Suggestion).

Liupold commented 4 years ago

Also I can't understand the changes in fdb921e. Are we running the preglue hook twice now?

My bad delete ta last preglue cmd. (Not needed).

Do you mean last as in "last in the code" or "last added"?

Last in code. (after swallowing).

Is there a reason for preglue happening before the swallowing?

yes, WM doesn't manage window after being swallowed.. (we can rename it as pre_swallow_hook or something similar).

SeerLite commented 4 years ago

yes, WM doesn't manage window after being swallowed.. (we can rename it as pre_swallow_hook or something similar).

Ah you're right, makes sense. I think it's best to keep it as just "preglue" because it only happens with --glue. And as for an actual "pre_swallow" it's already possible to just add it to swallow_command.

Liupold commented 4 years ago

yes, WM doesn't manage window after being swallowed.. (we can rename it as pre_swallow_hook or something similar).

Ah you're right, makes sense. I think it's best to keep it as just "preglue" because it only happens with --glue. And as for an actual "pre_swallow" it's already possible to just add it to swallow_command.

perfect!.

SeerLite commented 4 years ago

if you are working on the script you might want to use regex instead of grep in pgeo.

Do you mean using shell regex/parsing/whatever (${pgeo#etcetc})? Or just grep/sed but with regex? Using shell substring-thing is gonna need 2 assignments anyway (can't nest), and I'm not sure how to extract the information from the output to different variables with grep/tools.

Liupold commented 4 years ago

if you are working on the script you might want to use regex instead of grep in pgeo.

Do you mean using shell regex/parsing/whatever (${pgeo#etcetc})? Or just grep/sed but with regex? Using shell substring-thing is gonna need 2 assignments anyway (can't nest), and I'm not sure how to extract the information from the output to different variables with grep/tools.

Ideally shell, (multiple shell is better than grep).

SeerLite commented 4 years ago

Gotcha! I'm having some trouble figuring it out right now though. I can look into it tomorrow though

Liupold commented 4 years ago

Gotcha! I'm having some trouble figuring it out right now though. I can look into it tomorrow though

I just pushed (droped grep) you can look into that.

SeerLite commented 4 years ago

I can look into it tomorrow though-

I just pushed (droped grep

Or you can just figure it out by yourself like a boss lol

I'm fixing some capitalization to get it working... Nevermind then

SeerLite commented 4 years ago

(e4c7b89) Myself:

I'm not sure if using the shell approach is faster than the terminal one.
If it is, it might be a good idea to note that in the step too.

I did some tests with hyperfine (amazing tool):

$ hyperfine --warmup 3 --prepare 'pidswallow -v 0x08000002 || true' 'pidswallow -s 0x08000002' 'SHELL="" pidswallow -s 0x08000002'
Benchmark #1: pidswallow -s 0x08000002
  Time (mean ± σ):      62.9 ms ±   7.9 ms    [User: 22.2 ms, System: 27.9 ms]
  Range (min … max):    43.6 ms …  74.3 ms    20 runs

Benchmark #2: SHELL="" pidswallow -s 0x08000002
  Time (mean ± σ):      68.8 ms ±   8.6 ms    [User: 26.1 ms, System: 34.0 ms]
  Range (min … max):    55.2 ms …  87.2 ms    16 runs

Summary
  'pidswallow -s 0x08000002' ran
    1.09 ± 0.19 times faster than 'SHELL="" pidswallow -s 0x08000002'

Shell-based seems to be a bit faster than terminal-based. I'm guessing it has to do with this line. I'm gonna encourage users to use the shell-based approach in the readme. (It's nice to have the fallback though)

Liupold commented 4 years ago

I changed the suspected line, hope this will help, let me know if this helps.

Liupold commented 4 years ago

I changed the suspected line, hope this will help, let me know if this helps.

Benchmark #1: pidswallow -s 0x04800003
 ⠹ Performing warmup runs          ETA 00:00:0  Time (mean ± σ):      44.6 ms ±   7.3 ms    [User: 28.7 ms, System: 24.7 ms]
  Range (min … max):    31.6 ms …  59.2 ms    43 runs

Benchmark #2: SHELL="" pidswallow -s 0x04800003
  Time (mean ± σ):      46.3 ms ±   5.2 ms    [User: 29.9 ms, System: 25.0 ms]
  Range (min … max):    37.2 ms …  61.6 ms    33 runs

Summary
  'pidswallow -s 0x04800003' ran
    1.04 ± 0.21 times faster than 'SHELL="" pidswallow -s 0x04800003'
Liupold commented 4 years ago

you need to change the global variable. (Sorry, I don't know why i didn't thought of this before).

Liupold commented 4 years ago

NO Measurable change....

Benchmark #1: pidswallow -s 0x04800003
 ⠹ Performing warmup runs          ETA 00:00:0  Time (mean ± σ):      44.0 ms ±   6.5 ms    [User: 25.5 ms, System: 24.1 ms]
  Range (min … max):    32.0 ms …  57.9 ms    43 runs

Benchmark #2: SHELL="" pidswallow -s 0x04800003
  Time (mean ± σ):      48.4 ms ±   7.2 ms    [User: 28.0 ms, System: 25.7 ms]
  Range (min … max):    35.3 ms …  65.6 ms    37 runs

Summary
  'pidswallow -s 0x04800003' ran
    1.10 ± 0.23 times faster than 'SHELL="" pidswallow -s 0x04800003'
Liupold commented 4 years ago

Use dash...

Benchmark #1: pidswallow -s 0x02A00003
  Time (mean ± σ):      28.0 ms ±   4.3 ms    [User: 13.8 ms, System: 18.0 ms]
  Range (min … max):    24.1 ms …  48.0 ms    74 runs

Benchmark #2: SHELL="" pidswallow -s 0x02A00003
  Time (mean ± σ):      31.6 ms ±   4.5 ms    [User: 17.5 ms, System: 17.7 ms]
  Range (min … max):    25.7 ms …  45.5 ms    73 runs

Summary
  'pidswallow -s 0x02A00003' ran
    1.13 ± 0.24 times faster than 'SHELL="" pidswallow -s 0x02A00003'
SeerLite commented 4 years ago

Hey, sorry for the inactivity!

I changed the suspected line, hope this will help, let me know if this helps.

I actually thought it was fine how it was, but I see you've made some adjustments and now it's a bit faster (not sure how much though). Using dash is a great idea, but we're explicitly calling /bin/sh for subshells (not dash). Edit: Unless you're using a symlink of course.

Liupold commented 4 years ago

/bin/sh is actually a sym link. Point it to /use/bin/dash. (See dash arch wiki).

Liupold commented 4 years ago

I though it was good enough when I merged it, hope it was. If you get some time try using the floating wm , they should be improved. (Hopefully).

SeerLite commented 4 years ago

/bin/sh is actually a sym link. Point it to /use/bin/dash. (See dash arch wiki).

I actually don't like doing this, because I can never be sure if all the files in /usr/bin that start with #!/bin/sh are POSIX compliant (even if they should be). This is also stated in the archwiki:

You can re-symlink /bin/sh to /bin/dash, which can improve system performance, but first you must verify that none of the scripts that aren't explicitly #!/bin/bash require any of Bash's features and that all /bin/sh scripts are safely POSIX compliant.

It might be completely fine but I personally just use a custom #!/usr/bin/env dash shebang for the scripts I like to run quickly. I'm gonna be messing around with this stuff tomorrow though.

I though it was good enough when I merged it, hope it was. If you get some time try using the floating wm , they should be improved. (Hopefully).

Don't worry, it's been working great! I'll check out all the DEs I got tomorrow too. I really like the shell-based approach.