| Message ID | 20170926112803.4206-1-lvivier@redhat.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Fix output word | expand |
On 26/09/17 21:28, Laurent Vivier wrote: > We can select the console output, but it does not really work Can you please provide a QEMU command line which did not work before and does after this applied, just to verify I understand the change? The same question about "[PATCH] Use input-device and output-device". Thanks. > > Implement term-io-emit, as we have term-io-key to really > send characters to the output selected by stdout. > > Signed-off-by: Laurent Vivier <lvivier@redhat.com> > --- > slof/fs/term-io.fs | 14 ++++++++++++++ > 1 file changed, 14 insertions(+) > > diff --git a/slof/fs/term-io.fs b/slof/fs/term-io.fs > index 52ce12a..a0b0f4b 100644 > --- a/slof/fs/term-io.fs > +++ b/slof/fs/term-io.fs > @@ -40,6 +40,20 @@ > > 1 BUFFER: (term-io-char-buf) > > +: term-io-emit ( char -- ) > + s" stdout" get-chosen IF > + decode-int nip nip dup 0= IF 0 EXIT THEN > + swap (term-io-char-buf) c! > + >r (term-io-char-buf) 1 s" write" r@ $call-method > + drop > + r> drop > + ELSE > + [ ' emit behavior compile, ] > + THEN > +; > + > +' term-io-emit to emit > + > : term-io-key ( -- char ) > s" stdin" get-chosen IF > decode-int nip nip dup 0= IF 0 EXIT THEN >
On 26.09.2017 13:28, Laurent Vivier wrote: > We can select the console output, but it does not really work > > Implement term-io-emit, as we have term-io-key to really > send characters to the output selected by stdout. > > Signed-off-by: Laurent Vivier <lvivier@redhat.com> > --- > slof/fs/term-io.fs | 14 ++++++++++++++ > 1 file changed, 14 insertions(+) > > diff --git a/slof/fs/term-io.fs b/slof/fs/term-io.fs > index 52ce12a..a0b0f4b 100644 > --- a/slof/fs/term-io.fs > +++ b/slof/fs/term-io.fs > @@ -40,6 +40,20 @@ > > 1 BUFFER: (term-io-char-buf) > > +: term-io-emit ( char -- ) > + s" stdout" get-chosen IF > + decode-int nip nip dup 0= IF 0 EXIT THEN > + swap (term-io-char-buf) c! > + >r (term-io-char-buf) 1 s" write" r@ $call-method > + drop > + r> drop > + ELSE > + [ ' emit behavior compile, ] > + THEN > +; While this is basically the right direction, I've got three concerns / remarks: 1) Please don't use TABs in Forth code (see Coding Style in README) 2) Have you checked whether it also works with VGA screens? The code in slof/fs/display.fs ticks to EMIT again ... it looks like it should work, but better test it first... 3) Speed ... This rather code is called for every character that we print out. While it likely does not really matter for input, the output of text was always a rather critical thing in SLOF. Could you please do some speed measurements first? Something like: milliseconds 2000 0 do 41 emit loop milliseconds swap - cr .d cr ... and then compare the values before and after your change. Thanks, Thomas
On 27/09/2017 08:25, Alexey Kardashevskiy wrote: > On 26/09/17 21:28, Laurent Vivier wrote: >> We can select the console output, but it does not really work > > Can you please provide a QEMU command line which did not work before and > does after this applied, just to verify I understand the change? The same > question about "[PATCH] Use input-device and output-device". Thanks. > Start QEMU with two spapr-vty ports: qemu-system-ppc64 -nodefaults -nographic \ -chardev socket,id=serial_id_serial0,host=localhost,port=4441,telnet,server,wait \ -device spapr-vty,chardev=serial_id_serial0 \ -chardev socket,id=serial_id_serial1,host=localhost,port=4442,telnet,server,wait \ -device spapr-vty,chardev=serial_id_serial1 \ -monitor stdio Connect on the two ports with "telnet localhost 4441" and "telnet 4442". Once it is done you will see on the first console the SLOF logs. Then, in the first console: " /vdevice/vty@71000002" input Now what you write on second console is written on the first one. To send the characters to the second one, we should use: " /vdevice/vty@71000002" ouput but it doesn't work and this patch fixes that. A shortcut for these two commands is: " /vdevice/vty@71000002" io Thanks, Laurent >> >> Implement term-io-emit, as we have term-io-key to really >> send characters to the output selected by stdout. >> >> Signed-off-by: Laurent Vivier <lvivier@redhat.com> >> --- >> slof/fs/term-io.fs | 14 ++++++++++++++ >> 1 file changed, 14 insertions(+) >> >> diff --git a/slof/fs/term-io.fs b/slof/fs/term-io.fs >> index 52ce12a..a0b0f4b 100644 >> --- a/slof/fs/term-io.fs >> +++ b/slof/fs/term-io.fs >> @@ -40,6 +40,20 @@ >> >> 1 BUFFER: (term-io-char-buf) >> >> +: term-io-emit ( char -- ) >> + s" stdout" get-chosen IF >> + decode-int nip nip dup 0= IF 0 EXIT THEN >> + swap (term-io-char-buf) c! >> + >r (term-io-char-buf) 1 s" write" r@ $call-method >> + drop >> + r> drop >> + ELSE >> + [ ' emit behavior compile, ] >> + THEN >> +; >> + >> +' term-io-emit to emit >> + >> : term-io-key ( -- char ) >> s" stdin" get-chosen IF >> decode-int nip nip dup 0= IF 0 EXIT THEN >> > >
On 27/09/2017 09:25, Thomas Huth wrote: > On 26.09.2017 13:28, Laurent Vivier wrote: >> We can select the console output, but it does not really work >> >> Implement term-io-emit, as we have term-io-key to really >> send characters to the output selected by stdout. >> >> Signed-off-by: Laurent Vivier <lvivier@redhat.com> >> --- >> slof/fs/term-io.fs | 14 ++++++++++++++ >> 1 file changed, 14 insertions(+) >> >> diff --git a/slof/fs/term-io.fs b/slof/fs/term-io.fs >> index 52ce12a..a0b0f4b 100644 >> --- a/slof/fs/term-io.fs >> +++ b/slof/fs/term-io.fs >> @@ -40,6 +40,20 @@ >> >> 1 BUFFER: (term-io-char-buf) >> >> +: term-io-emit ( char -- ) >> + s" stdout" get-chosen IF >> + decode-int nip nip dup 0= IF 0 EXIT THEN >> + swap (term-io-char-buf) c! >> + >r (term-io-char-buf) 1 s" write" r@ $call-method >> + drop >> + r> drop >> + ELSE >> + [ ' emit behavior compile, ] >> + THEN >> +; > > While this is basically the right direction, I've got three concerns / > remarks: > > 1) Please don't use TABs in Forth code (see Coding Style in README) Yes, sorry, I've seen that after having sent the patch. > > 2) Have you checked whether it also works with VGA screens? The code in > slof/fs/display.fs ticks to EMIT again ... it looks like it should > work, but better test it first... In fact, it doesn't work... The serial console is mirrored to the VGA display, and when we try to switch to the second serial console, only the input moves (output stays on the first console and VGA). > > 3) Speed ... This rather code is called for every character that we > print out. While it likely does not really matter for input, the > output of text was always a rather critical thing in SLOF. Could > you please do some speed measurements first? Something like: > > milliseconds 2000 0 do 41 emit loop milliseconds swap - cr .d cr > > ... and then compare the values before and after your change. Without: 797 796 807 With: 4772 4710 4782 So it's 6 times slower... what can we do for that? Thanks, Laurent
Laurent Vivier <lvivier@redhat.com> writes: > We can select the console output, but it does not really work > > Implement term-io-emit, as we have term-io-key to really > send characters to the output selected by stdout. > > Signed-off-by: Laurent Vivier <lvivier@redhat.com> > --- > slof/fs/term-io.fs | 14 ++++++++++++++ > 1 file changed, 14 insertions(+) > > diff --git a/slof/fs/term-io.fs b/slof/fs/term-io.fs > index 52ce12a..a0b0f4b 100644 > --- a/slof/fs/term-io.fs > +++ b/slof/fs/term-io.fs > @@ -40,6 +40,20 @@ > > 1 BUFFER: (term-io-char-buf) > > +: term-io-emit ( char -- ) > + s" stdout" get-chosen IF > + decode-int nip nip dup 0= IF 0 EXIT THEN > + swap (term-io-char-buf) c! > + >r (term-io-char-buf) 1 s" write" r@ $call-method > + drop > + r> drop Indentation problem here. > + ELSE > + [ ' emit behavior compile, ] > + THEN > +; > + > +' term-io-emit to emit > + > : term-io-key ( -- char ) > s" stdin" get-chosen IF > decode-int nip nip dup 0= IF 0 EXIT THEN Apart from the above nit, looks fine. Reviewed-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> Regards, Nikunj
Hi all, On Wed, Sep 27, 2017 at 09:25:04AM +0200, Thomas Huth wrote: > On 26.09.2017 13:28, Laurent Vivier wrote: > > diff --git a/slof/fs/term-io.fs b/slof/fs/term-io.fs > > index 52ce12a..a0b0f4b 100644 > > --- a/slof/fs/term-io.fs > > +++ b/slof/fs/term-io.fs > > @@ -40,6 +40,20 @@ > > > > 1 BUFFER: (term-io-char-buf) > > > > +: term-io-emit ( char -- ) > > + s" stdout" get-chosen IF > > + decode-int nip nip dup 0= IF 0 EXIT THEN > > + swap (term-io-char-buf) c! > > + >r (term-io-char-buf) 1 s" write" r@ $call-method > > + drop > > + r> drop > > + ELSE > > + [ ' emit behavior compile, ] > > + THEN > > +; First some trivialities... Things are easier to read if you use early-return instead of big IF...ELSE...THEN blocks. R> DROP is a red flag usually. So something like : term-io-emit ( char -- ) s" stdout" get-chosen 0= IF [ ' emit behavior compile, ] THEN decode-int nip nip dup 0= IF 0 EXIT THEN swap (term-io-char-buf) c! >r (term-io-char-buf) 1 s" write" r> $call-method drop ; > 3) Speed ... This rather code is called for every character that we > print out. While it likely does not really matter for input, the > output of text was always a rather critical thing in SLOF. Is it? I always found the output devices to be *much* slower than the code, heh. Or everything was so fast it did not matter. But there are two big problems with the current architecture: 1) You should not use get-chosen. Only SLOF itself is allowed to modify anything in /chosen (directly), so the "output" word can just scribble the "stdout" ihandle into some variable it can access much faster. Better yet, it can store some xt from the chosen stdout package (the "write" one, for example). 2) EMIT in a loop is much slower than TYPE on some devices (say, if you do some hypervisor call for output, or on a network device or similar where you send a packet per character this way instead of the whole string at once). So you shouldn't use EMIT so much... If speed is still bad after 1), replace (simplified :-) ) : emit ... ; : type bounds ?DO i c@ emit LOOP ; with : type ... ; CVARIABLE emit-buf : emit emit-buf c! emit-buf 1 type ; Segher
On 27/09/17 18:10, Laurent Vivier wrote: > On 27/09/2017 09:25, Thomas Huth wrote: >> On 26.09.2017 13:28, Laurent Vivier wrote: >>> We can select the console output, but it does not really work >>> >>> Implement term-io-emit, as we have term-io-key to really >>> send characters to the output selected by stdout. >>> >>> Signed-off-by: Laurent Vivier <lvivier@redhat.com> >>> --- >>> slof/fs/term-io.fs | 14 ++++++++++++++ >>> 1 file changed, 14 insertions(+) >>> >>> diff --git a/slof/fs/term-io.fs b/slof/fs/term-io.fs >>> index 52ce12a..a0b0f4b 100644 >>> --- a/slof/fs/term-io.fs >>> +++ b/slof/fs/term-io.fs >>> @@ -40,6 +40,20 @@ >>> >>> 1 BUFFER: (term-io-char-buf) >>> >>> +: term-io-emit ( char -- ) >>> + s" stdout" get-chosen IF >>> + decode-int nip nip dup 0= IF 0 EXIT THEN >>> + swap (term-io-char-buf) c! >>> + >r (term-io-char-buf) 1 s" write" r@ $call-method >>> + drop >>> + r> drop >>> + ELSE >>> + [ ' emit behavior compile, ] >>> + THEN >>> +; >> >> While this is basically the right direction, I've got three concerns / >> remarks: >> >> 1) Please don't use TABs in Forth code (see Coding Style in README) > > Yes, sorry, I've seen that after having sent the patch. > >> >> 2) Have you checked whether it also works with VGA screens? The code in >> slof/fs/display.fs ticks to EMIT again ... it looks like it should >> work, but better test it first... > > In fact, it doesn't work... The serial console is mirrored to the VGA > display, and when we try to switch to the second serial console, only > the input moves (output stays on the first console and VGA). So, v2 is coming, right? > >> >> 3) Speed ... This rather code is called for every character that we >> print out. While it likely does not really matter for input, the >> output of text was always a rather critical thing in SLOF. Could >> you please do some speed measurements first? Something like: >> >> milliseconds 2000 0 do 41 emit loop milliseconds swap - cr .d cr >> >> ... and then compare the values before and after your change. > > Without: > > 797 796 807 > > With: > > 4772 4710 4782 > > So it's 6 times slower... what can we do for that? > > Thanks, > Laurent > _______________________________________________ > SLOF mailing list > SLOF@lists.ozlabs.org > https://lists.ozlabs.org/listinfo/slof >
On 04/10/2017 10:33, Alexey Kardashevskiy wrote: > On 27/09/17 18:10, Laurent Vivier wrote: >> On 27/09/2017 09:25, Thomas Huth wrote: >>> On 26.09.2017 13:28, Laurent Vivier wrote: >>>> We can select the console output, but it does not really work >>>> >>>> Implement term-io-emit, as we have term-io-key to really >>>> send characters to the output selected by stdout. >>>> >>>> Signed-off-by: Laurent Vivier <lvivier@redhat.com> >>>> --- >>>> slof/fs/term-io.fs | 14 ++++++++++++++ >>>> 1 file changed, 14 insertions(+) >>>> >>>> diff --git a/slof/fs/term-io.fs b/slof/fs/term-io.fs >>>> index 52ce12a..a0b0f4b 100644 >>>> --- a/slof/fs/term-io.fs >>>> +++ b/slof/fs/term-io.fs >>>> @@ -40,6 +40,20 @@ >>>> >>>> 1 BUFFER: (term-io-char-buf) >>>> >>>> +: term-io-emit ( char -- ) >>>> + s" stdout" get-chosen IF >>>> + decode-int nip nip dup 0= IF 0 EXIT THEN >>>> + swap (term-io-char-buf) c! >>>> + >r (term-io-char-buf) 1 s" write" r@ $call-method >>>> + drop >>>> + r> drop >>>> + ELSE >>>> + [ ' emit behavior compile, ] >>>> + THEN >>>> +; >>> >>> While this is basically the right direction, I've got three concerns / >>> remarks: >>> >>> 1) Please don't use TABs in Forth code (see Coding Style in README) >> >> Yes, sorry, I've seen that after having sent the patch. >> >>> >>> 2) Have you checked whether it also works with VGA screens? The code in >>> slof/fs/display.fs ticks to EMIT again ... it looks like it should >>> work, but better test it first... >> >> In fact, it doesn't work... The serial console is mirrored to the VGA >> display, and when we try to switch to the second serial console, only >> the input moves (output stays on the first console and VGA). > > > So, v2 is coming, right? Yes, but I need some time to work on it... Thanks, Laurent > >> >>> >>> 3) Speed ... This rather code is called for every character that we >>> print out. While it likely does not really matter for input, the >>> output of text was always a rather critical thing in SLOF. Could >>> you please do some speed measurements first? Something like: >>> >>> milliseconds 2000 0 do 41 emit loop milliseconds swap - cr .d cr >>> >>> ... and then compare the values before and after your change. >> >> Without: >> >> 797 796 807 >> >> With: >> >> 4772 4710 4782 >> >> So it's 6 times slower... what can we do for that? >> >> Thanks, >> Laurent >> _______________________________________________ >> SLOF mailing list >> SLOF@lists.ozlabs.org >> https://lists.ozlabs.org/listinfo/slof >> > >
diff --git a/slof/fs/term-io.fs b/slof/fs/term-io.fs index 52ce12a..a0b0f4b 100644 --- a/slof/fs/term-io.fs +++ b/slof/fs/term-io.fs @@ -40,6 +40,20 @@ 1 BUFFER: (term-io-char-buf) +: term-io-emit ( char -- ) + s" stdout" get-chosen IF + decode-int nip nip dup 0= IF 0 EXIT THEN + swap (term-io-char-buf) c! + >r (term-io-char-buf) 1 s" write" r@ $call-method + drop + r> drop + ELSE + [ ' emit behavior compile, ] + THEN +; + +' term-io-emit to emit + : term-io-key ( -- char ) s" stdin" get-chosen IF decode-int nip nip dup 0= IF 0 EXIT THEN
We can select the console output, but it does not really work Implement term-io-emit, as we have term-io-key to really send characters to the output selected by stdout. Signed-off-by: Laurent Vivier <lvivier@redhat.com> --- slof/fs/term-io.fs | 14 ++++++++++++++ 1 file changed, 14 insertions(+)