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

input is dropped from large redirections #2548

Closed
dankamongmen opened this issue Jan 10, 2022 · 14 comments
Closed

input is dropped from large redirections #2548

dankamongmen opened this issue Jan 10, 2022 · 14 comments
Assignees
Labels
bug Something isn't working input readin' dem bytes
Milestone

Comments

@dankamongmen
Copy link
Owner

Try running e.g. (cat ../src/lib/*.c) | ./notcurses-input. It'll report a lot of input errors at the end, which I'm pretty certain it didn't do before (i'm checking 3.0.4 now).

@dankamongmen dankamongmen added bug Something isn't working input readin' dem bytes labels Jan 10, 2022
@dankamongmen dankamongmen added this to the 3.1.0 milestone Jan 10, 2022
@dankamongmen dankamongmen self-assigned this Jan 10, 2022
@dankamongmen
Copy link
Owner Author

nope, this seems to happen with 3.0.4 as well =.

they're not dropped at the input side, but internally. we ought be strong against this (we ought just wait on the client input buffer).

@dankamongmen
Copy link
Owner Author

so yeah, it starts right at 8192 bytes (coincidentally the size of our input buffers). testing with dd:

[schwarzgerat](0) $ dd if=/dev/zero of=/dev/stdout bs=8192 count=1 | ./notcurses-input 
1+0 records in
1+0 records out
8192 bytes (8.2 kB, 8.0 KiB) copied, 4.7908e-05 s, 171 MB/s
2 renders, 423.95µs (142.76µs min, 211.97µs avg, 281.18µs max)
2 rasters, 132.21µs (45.54µs min, 66.10µs avg, 86.67µs max)
2 writes, 112.14µs (40.79µs min, 56.07µs avg, 71.35µs max)
894B (0B min, 447B avg, 894B max) 8193 inputs Ghpa: 0
0 failed renders, 0 failed rasters, 0 refreshes, 0 input errors
RGB emits:elides: def 1:48 fg 8:98 bg 1:56
Cell emits:elides: 106:30565 (99.65%) 97.96% 92.45% 98.25%
Bmap emits:elides: 1:1 (50.00%) 517B (57.83%) SuM: 0 (0.00%)
[schwarzgerat](0) $ dd if=/dev/zero of=/dev/stdout bs=32768 count=1 | ./notcurses-input 
1+0 records in
1+0 records out
32768 bytes (33 kB, 32 KiB) copied, 4.9449e-05 s, 663 MB/s
2 renders, 422.54µs (144.28µs min, 211.27µs avg, 278.26µs max)
2 rasters, 135.56µs (45.82µs min, 67.78µs avg, 89.74µs max)
2 writes, 112.38µs (40.52µs min, 56.19µs avg, 71.86µs max)
894B (0B min, 447B avg, 894B max) 32769 inputs Ghpa: 0
0 failed renders, 0 failed rasters, 0 refreshes, 24576 input errors
RGB emits:elides: def 1:48 fg 8:98 bg 1:56
Cell emits:elides: 106:30565 (99.65%) 97.96% 92.45% 98.25%
Bmap emits:elides: 1:1 (50.00%) 517B (57.83%) SuM: 0 (0.00%)
[schwarzgerat](0) $ 

@dankamongmen
Copy link
Owner Author

[schwarzgerat](0) $ dd if=/dev/zero of=/dev/stdout bs=1024 count=9 | ./notcurses-input 
9+0 records in
9+0 records out
9216 bytes (9.2 kB, 9.0 KiB) copied, 5.1779e-05 s, 178 MB/s
2 renders, 422.81µs (142.26µs min, 211.40µs avg, 280.54µs max)
2 rasters, 132.50µs (45.99µs min, 66.25µs avg, 86.51µs max)
2 writes, 112.40µs (41.34µs min, 56.20µs avg, 71.06µs max)
894B (0B min, 447B avg, 894B max) 9217 inputs Ghpa: 0
0 failed renders, 0 failed rasters, 0 refreshes, 1024 input errors
RGB emits:elides: def 1:48 fg 8:98 bg 1:56
Cell emits:elides: 106:30565 (99.65%) 97.96% 92.45% 98.25%
Bmap emits:elides: 1:1 (50.00%) 517B (57.83%) SuM: 0 (0.00%)
[schwarzgerat](0) $ 

@dankamongmen
Copy link
Owner Author

dankamongmen commented Jan 12, 2022

alright, unwinding from the core:

  • load_ncinput() is the fundamental place where this all goes down.
    • if we get here, we need to have empty space in inputs (ivalid < isize), or we're throwing it away
    • note that this is also where we synthesize signals, which we want to happen ASAP
  • to handle a keyboard-initiated signal, we need always be reading from the terminal and processing those reads
  • but we do not need to always be reading from a non-terminal stdin

the inner read loop looks like:

  • read_inputs_nblock()
    • block_on_input() -- waits for input readiness on stdin if ibuf isn't full, ipipe if it is. waits on termfd if that's defined.
    • read_input_nblock() on termfd if termfd was active
    • read_input_nblock() on stdin if stdin was active
  • process_ibuf()
    • load NCKEY_SIGNAL/NCKEY_RESIZE if they happened (can be dropped)
    • if tbuf is occupied, run process_escapes() on tbuf. tbuf is terminal when stdin is not.
      • move any leftovers to the front; they might have been a partial escape
    • if ibuf is occupied and is independent, run process_bulk()
      • move any leftovers to the front
    • if ibuf is occupied and also terminal, run process_melange()
      • move any leftovers to the front; they might have been a partial escape

so we only need to be processing to the end in process_melange(), since that might be terminal responses or signals. process_bulk() can stop when the output buffer is full, and absolutely should. let's see what it's doing...

@dankamongmen
Copy link
Owner Author

so i think we need check for available space all the way back in process_bulk(), bailing there before calling process_ncinput(), as the latter is also called by process_melange() (which must process all its input). but we'll have to be careful. if we are full, how do we ensure we start distributing, once some are made available?

@dankamongmen
Copy link
Owner Author

dankamongmen commented Jan 12, 2022

so if we drop out from process_bulk()'s loop early when ivalid == isize, and move everything to the front of ibuf. we circle around, and if there's anything to read, we fill up the ibuf. if the client eats some during this time, we could proceed immediately. if the client eats everything in its buffer, and no new input has been generated, how do we ensure the existing unprocessed input gets to the client buffers? if we're full, what happens?

@dankamongmen
Copy link
Owner Author

we get a signal from the client when it reads from its buffer, providing us the space necessary to process input. so we ought be fine so long as we check the number free, see there are none, and enter the poll(). so we get woken up either for our input, or theirs. we ought be safe.

@dankamongmen
Copy link
Owner Author

well, that appears to fix it, as expected...let's think this through some more.

@dankamongmen
Copy link
Owner Author

hrmmm, still a few errors, but far fewer:

267.90MiB (894B min, 11.02KiB avg, 14.56KiB max) 32816 inputs Ghpa: 12
0 failed renders, 0 failed rasters, 0 refreshes, 623 input errors

@dankamongmen
Copy link
Owner Author

down further, and we've eliminated a bunch of busy work:

269.57MiB (0B min, 11.08KiB avg, 14.76KiB max) 32291 inputs Ghpa: 12
0 failed renders, 0 failed rasters, 0 refreshes, 7 input errors

@dankamongmen
Copy link
Owner Author

3.04GiB (0B min, 13.68KiB avg, 18.70KiB max) 235160 inputs Ghpa: 189
0 failed renders, 0 failed rasters, 0 refreshes, 2109 input errors

@dankamongmen
Copy link
Owner Author

the error i'm seeing is happening at the beginning:

process_escape:2118:walk result on 116 (t): 2 274                                                                                   
process_escape:2106:initialized automaton to 1                                                                                      
process_escape:2118:walk result on 91 ([): 0 4                                                                                      
process_escape:2118:walk result on 63 (?): 0 304                                                                                    
process_escape:2118:walk result on 54 (6): 0 346                                                                                    
process_escape:2118:walk result on 50 (2): 0 305                                                                                    
process_escape:2118:walk result on 59 (;): 0 357                                                                                    
da1_attrs_cb:1173:read primary device attributes                                                                                    
da1_attrs_cb:1176:DA1: 62 []                                                                                                        
process_escape:2118:walk result on 99 (c): 1 358                                                                                    
handoff_initial_responses_late:1119:handing off initial responses                                                                   
block_on_input:2356:blocking on input availability                                                                                  
block_on_input:2401:output queues full; blocking on ipipes                                                                          
block_on_input:2413:waiting on 2 fds (ibuf: 8192/8192)                                                                              
ncplane_new_internal:671:created new 60x128 plane "std" @ 0x0                                                                       
ncplane_new_internal:671:created new 6x56 plane "plot" @ 53x36                                                                      
ncplane_new_internal:671:created new 6x56 plane "plot" @ 53x36                                                                      
ncplane_erase:2385:erasing 6x56 plane                                                                                               
ncvisual_blit:1116:inblit 132x616 0@0 0x0 @ 0x0 0x56333ad16a80                                                                      
ncvisual_geom_inner:277:vis 0x0+0x0 0x7f70ccc42010                                                                                  
ncvisual_geom_inner:370:pixel prescale: 132 616 132 616                                                                             
ncvisual_geom_inner:385:pblit: 132x6160x0 of 132/616 stride 2496 @0x0 0x7f70ccc42010 11                                          
ncvisual_geom_inner:445:rgeom: 6 56 132 616 @ 0/0 (6 on 0x56333ad16a80)                                                             
ncvisual_blit:1165:blit to plane 0x56333ad16a80 at 53/36 geom 6x56                                                                  
ncvisual_render_pixels:1027:pblit: rows/cols: 6x56 plane: 6/56                                                                      
block_on_input:2440:poll returned 1                                                                                                 
block_on_input:2458:got events: Ti                                                                                                  
read_input_nblock:2069:read 9B from 8 (8183B left)                                                                                  
process_escape:2106:initialized automaton to 1                                                                                      
process_escape:2118:walk result on 91 ([): 0 4                                                                                      
process_escape:2118:walk result on 49 (1): 0 24                                                                                     
process_escape:2118:walk result on 51 (3): 0 269                                                                                    
process_escape:2118:walk result on 59 (;): 0 270                                                                                    
process_escape:2118:walk result on 49 (1): 0 271                                                                                    
process_escape:2118:walk result on 58 (:): 0 278                                                                                    
process_escape:2118:walk result on 51 (3): 0 279                                                                                    
kitty_kbd:797:v/m/e 1115121 1 3                                                                                                     
load_ncinput:518:dropping input 0x001103f1                                                                                          
inc_input_errors:123:ZORP ZORP ZORP ZORP ZORP        

@dankamongmen
Copy link
Owner Author

so we start off in this situation:

esctrie_make_string:181:made string: 450                                                                                            
block_on_input:2356:blocking on input availability                                                                                  
block_on_input:2413:waiting on 2 fds (ibuf: 0/8192)                                                                                 
block_on_input:2440:poll returned 2                                                                                                 
block_on_input:2458:got events: TI                                                                                                  
read_input_nblock:2069:read 4095B from 8 (4097B left)                                                                               
read_input_nblock:2069:read 8192B from 0 (0B left)                                                                                  
process_escape:2106:initialized automaton to 1                                                                                      
process_escape:2118:walk result on 91 ([): 0 4                                                                                      
process_escape:2118:walk result on 49 (1): 0 24                                                                                     
process_escape:2118:walk result on 59 (;): 0 41                                                                                     
process_escape:2118:walk result on 49 (1): 0 271                                                                                    
process_escape:2118:walk result on 82 (R): 2 323                                                                                    
process_escape:2106:initialized automaton to 1

@dankamongmen
Copy link
Owner Author

ok, so that's the enter+release at the end of a kitty command line, and then mouse movement events. this is all stuff coming in over the true terminal fd, which we do indeed have to read through. that's not a problem. this is fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working input readin' dem bytes
Projects
None yet
Development

No branches or pull requests

1 participant