diff mbox

Do not include the FCode evaluator by default anymore

Message ID 1472612414-2382-1-git-send-email-thuth@redhat.com
State Accepted
Headers show

Commit Message

Thomas Huth Aug. 31, 2016, 3 a.m. UTC
Commit 2fed5652819ad26627a8 ("Always include evaluator, move
framebuffer token init to fbuffer.fs") made sure that the FCode
evaluator is always included, during each boot cycle. The basic
idea was that we would soon be starting to support PCI cards with
FCode drivers on them. However, this has never happened, and so
this change was in vain. The bad thing is now that the inclusion
of the FCode evaluator also takes a lot of precious boot time,
e.g. when running in QEMU TCG mode, it is more than a second.
So to be able to boot faster again, disable the FCode evaluator
by default again and put it into the ROM-fs instead (so it still
can be loaded manually with "include evaluator.fs" if necessary).

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 board-js2x/slof/Makefile |  1 +
 board-js2x/slof/OF.fs    |  4 ---
 board-js2x/slof/tree.fs  |  7 ++++
 board-qemu/slof/Makefile |  1 +
 board-qemu/slof/OF.fs    |  4 ---
 slof/fs/fbuffer.fs       | 54 -----------------------------
 slof/fs/fcode/tokens.fs  | 90 ++++++++++++++++++++++++------------------------
 7 files changed, 54 insertions(+), 107 deletions(-)

Comments

Nikunj A Dadhania Aug. 31, 2016, 5:21 a.m. UTC | #1
Thomas Huth <thuth@redhat.com> writes:

> Commit 2fed5652819ad26627a8 ("Always include evaluator, move
> framebuffer token init to fbuffer.fs") made sure that the FCode
> evaluator is always included, during each boot cycle. The basic
> idea was that we would soon be starting to support PCI cards with
> FCode drivers on them. However, this has never happened, and so
> this change was in vain. The bad thing is now that the inclusion
> of the FCode evaluator also takes a lot of precious boot time,
> e.g. when running in QEMU TCG mode, it is more than a second.
> So to be able to boot faster again, disable the FCode evaluator
> by default again and put it into the ROM-fs instead (so it still
> can be loaded manually with "include evaluator.fs" if necessary).
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>

Reviewed-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>

Regards
Nikunj
Segher Boessenkool Aug. 31, 2016, 5:44 a.m. UTC | #2
On Wed, Aug 31, 2016 at 05:00:14AM +0200, Thomas Huth wrote:
> Commit 2fed5652819ad26627a8 ("Always include evaluator, move
> framebuffer token init to fbuffer.fs") made sure that the FCode
> evaluator is always included, during each boot cycle. The basic
> idea was that we would soon be starting to support PCI cards with
> FCode drivers on them. However, this has never happened, and so
> this change was in vain. The bad thing is now that the inclusion
> of the FCode evaluator also takes a lot of precious boot time,
> e.g. when running in QEMU TCG mode, it is more than a second.

Why is it so slow?


Segher
Thomas Huth Aug. 31, 2016, 7:02 a.m. UTC | #3
On 31.08.2016 07:44, Segher Boessenkool wrote:
> On Wed, Aug 31, 2016 at 05:00:14AM +0200, Thomas Huth wrote:
>> Commit 2fed5652819ad26627a8 ("Always include evaluator, move
>> framebuffer token init to fbuffer.fs") made sure that the FCode
>> evaluator is always included, during each boot cycle. The basic
>> idea was that we would soon be starting to support PCI cards with
>> FCode drivers on them. However, this has never happened, and so
>> this change was in vain. The bad thing is now that the inclusion
>> of the FCode evaluator also takes a lot of precious boot time,
>> e.g. when running in QEMU TCG mode, it is more than a second.
> 
> Why is it so slow?

The time is lost in token.fs. I think it's because it is using a lot of
TICKs to set the behavior of the various tokens, and that seems to slow
things down.

 Thomas
Segher Boessenkool Aug. 31, 2016, 8:59 a.m. UTC | #4
On Wed, Aug 31, 2016 at 09:02:20AM +0200, Thomas Huth wrote:
> On 31.08.2016 07:44, Segher Boessenkool wrote:
> > On Wed, Aug 31, 2016 at 05:00:14AM +0200, Thomas Huth wrote:
> >> Commit 2fed5652819ad26627a8 ("Always include evaluator, move
> >> framebuffer token init to fbuffer.fs") made sure that the FCode
> >> evaluator is always included, during each boot cycle. The basic
> >> idea was that we would soon be starting to support PCI cards with
> >> FCode drivers on them. However, this has never happened, and so
> >> this change was in vain. The bad thing is now that the inclusion
> >> of the FCode evaluator also takes a lot of precious boot time,
> >> e.g. when running in QEMU TCG mode, it is more than a second.
> > 
> > Why is it so slow?
> 
> The time is lost in token.fs. I think it's because it is using a lot of
> TICKs to set the behavior of the various tokens, and that seems to slow
> things down.

So name lookup is still too slow?  It is using a very simplistic cache
that sped it up >100x when I added it (was using a slow sim then, too),
but that cache is killed every time you change search order, etc.

I'll have a look at token.fs .


Segher
Segher Boessenkool Sept. 1, 2016, 2:27 p.m. UTC | #5
On Wed, Aug 31, 2016 at 03:59:59AM -0500, Segher Boessenkool wrote:
> On Wed, Aug 31, 2016 at 09:02:20AM +0200, Thomas Huth wrote:
> > On 31.08.2016 07:44, Segher Boessenkool wrote:
> > > On Wed, Aug 31, 2016 at 05:00:14AM +0200, Thomas Huth wrote:
> > >> Commit 2fed5652819ad26627a8 ("Always include evaluator, move
> > >> framebuffer token init to fbuffer.fs") made sure that the FCode
> > >> evaluator is always included, during each boot cycle. The basic
> > >> idea was that we would soon be starting to support PCI cards with
> > >> FCode drivers on them. However, this has never happened, and so
> > >> this change was in vain. The bad thing is now that the inclusion
> > >> of the FCode evaluator also takes a lot of precious boot time,
> > >> e.g. when running in QEMU TCG mode, it is more than a second.
> > > 
> > > Why is it so slow?
> > 
> > The time is lost in token.fs. I think it's because it is using a lot of
> > TICKs to set the behavior of the various tokens, and that seems to slow
> > things down.
> 
> So name lookup is still too slow?  It is using a very simplistic cache
> that sped it up >100x when I added it (was using a slow sim then, too),
> but that cache is killed every time you change search order, etc.
> 
> I'll have a look at token.fs .

I don't see anything else that could cause the slowdown, just all the
ticks.

One way to speed this up is to put the init in a word (so use ['] instead),
it probably is quite noticably faster.  Fast enough, I do not know.


Segher
Thomas Huth Sept. 1, 2016, 3:53 p.m. UTC | #6
On 01.09.2016 16:27, Segher Boessenkool wrote:
> On Wed, Aug 31, 2016 at 03:59:59AM -0500, Segher Boessenkool wrote:
>> On Wed, Aug 31, 2016 at 09:02:20AM +0200, Thomas Huth wrote:
>>> On 31.08.2016 07:44, Segher Boessenkool wrote:
>>>> On Wed, Aug 31, 2016 at 05:00:14AM +0200, Thomas Huth wrote:
>>>>> Commit 2fed5652819ad26627a8 ("Always include evaluator, move
>>>>> framebuffer token init to fbuffer.fs") made sure that the FCode
>>>>> evaluator is always included, during each boot cycle. The basic
>>>>> idea was that we would soon be starting to support PCI cards with
>>>>> FCode drivers on them. However, this has never happened, and so
>>>>> this change was in vain. The bad thing is now that the inclusion
>>>>> of the FCode evaluator also takes a lot of precious boot time,
>>>>> e.g. when running in QEMU TCG mode, it is more than a second.
>>>>
>>>> Why is it so slow?
>>>
>>> The time is lost in token.fs. I think it's because it is using a lot of
>>> TICKs to set the behavior of the various tokens, and that seems to slow
>>> things down.
>>
>> So name lookup is still too slow?  It is using a very simplistic cache
>> that sped it up >100x when I added it (was using a slow sim then, too),
>> but that cache is killed every time you change search order, etc.
>>
>> I'll have a look at token.fs .
> 
> I don't see anything else that could cause the slowdown, just all the
> ticks.
> 
> One way to speed this up is to put the init in a word (so use ['] instead),
> it probably is quite noticably faster.  Fast enough, I do not know.

I tried that already, but it's also not fast enough - it still slows
down the boot process by more than a second.
Since we're currently not using the FCODE evaluator at all, I think it
is not worth the effort to spend a lot of time with optimizations here
... let's simply disable it by default 'till we really need it again.

 Thomas
Segher Boessenkool Sept. 1, 2016, 4:06 p.m. UTC | #7
On Thu, Sep 01, 2016 at 05:53:21PM +0200, Thomas Huth wrote:
> > One way to speed this up is to put the init in a word (so use ['] instead),
> > it probably is quite noticably faster.  Fast enough, I do not know.
> 
> I tried that already, but it's also not fast enough - it still slows
> down the boot process by more than a second.

Hrm pity, I wonder what is going on then.

> Since we're currently not using the FCODE evaluator at all, I think it
> is not worth the effort to spend a lot of time with optimizations here
> ... let's simply disable it by default 'till we really need it again.

Sure, that is a good stopgap.  My point here is that this points at an
inefficiency in the engine somewhere, and everything will benefit from
fixing that.


Segher
Alexey Kardashevskiy Sept. 14, 2016, 6:57 a.m. UTC | #8
On 31/08/16 13:00, Thomas Huth wrote:
> Commit 2fed5652819ad26627a8 ("Always include evaluator, move
> framebuffer token init to fbuffer.fs") made sure that the FCode
> evaluator is always included, during each boot cycle. The basic
> idea was that we would soon be starting to support PCI cards with
> FCode drivers on them. However, this has never happened, and so
> this change was in vain. The bad thing is now that the inclusion
> of the FCode evaluator also takes a lot of precious boot time,
> e.g. when running in QEMU TCG mode, it is more than a second.
> So to be able to boot faster again, disable the FCode evaluator
> by default again and put it into the ROM-fs instead (so it still
> can be loaded manually with "include evaluator.fs" if necessary).
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>

Thanks, applied.
diff mbox

Patch

diff --git a/board-js2x/slof/Makefile b/board-js2x/slof/Makefile
index 4cdd5fa..d441b5a 100644
--- a/board-js2x/slof/Makefile
+++ b/board-js2x/slof/Makefile
@@ -89,6 +89,7 @@  OF_FFS_FILES = \
 	$(SLOFCMNDIR)/fs/pci-config-bridge.fs \
 	$(SLOFCMNDIR)/fs/update_flash.fs \
 	$(SLOFCMNDIR)/fs/xmodem.fs \
+	$(SLOFCMNDIR)/fs/fcode/evaluator.fs \
 	$(SLOFCMNDIR)/fs/devices/pci-device_10de_0141.fs \
 	$(SLOFCMNDIR)/fs/devices/pci-class_02.fs \
 	$(SLOFBRDDIR)/default-font.bin
diff --git a/board-js2x/slof/OF.fs b/board-js2x/slof/OF.fs
index 3e37735..a51f473 100644
--- a/board-js2x/slof/OF.fs
+++ b/board-js2x/slof/OF.fs
@@ -209,10 +209,6 @@  d# 14318378 VALUE tb-frequency   \ default value - needed for "ms" to work
 
 #include <timebase.fs>
 
-270 cp
-
-#include <fcode/evaluator.fs>
-
 280 cp
 
 \ rtas-config is not used
diff --git a/board-js2x/slof/tree.fs b/board-js2x/slof/tree.fs
index 040d99f..1f924ea 100644
--- a/board-js2x/slof/tree.fs
+++ b/board-js2x/slof/tree.fs
@@ -140,6 +140,13 @@  set-node
 
 #include "ht.fs"
 
+6a0 cp
+get-node device-end
+\ At this point the SAS controller has been detected and we know if it
+\ is a bimini or js21. If it is bimini the fcode evaluator is included
+bimini? ?include evaluator.fs
+set-node
+
 6b0 cp
 
 u4? ?include attu.fs
diff --git a/board-qemu/slof/Makefile b/board-qemu/slof/Makefile
index 9cd6c0a..940a15a 100644
--- a/board-qemu/slof/Makefile
+++ b/board-qemu/slof/Makefile
@@ -101,6 +101,7 @@  OF_FFS_FILES = \
 	$(SLOFCMNDIR)/fs/scsi-host-helpers.fs \
 	$(SLOFCMNDIR)/fs/scsi-probe-helpers.fs \
 	$(SLOFCMNDIR)/fs/scsi-support.fs \
+	$(SLOFCMNDIR)/fs/fcode/evaluator.fs \
 	$(SLOFBRDDIR)/default-font.bin \
 	$(SLOFBRDDIR)/pci-phb.fs \
 	$(SLOFBRDDIR)/rtas.fs \
diff --git a/board-qemu/slof/OF.fs b/board-qemu/slof/OF.fs
index 784f2bd..5a4368f 100644
--- a/board-qemu/slof/OF.fs
+++ b/board-qemu/slof/OF.fs
@@ -93,10 +93,6 @@  d# 512000000 VALUE tb-frequency   \ default value - needed for "ms" to work
 
 #include <timebase.fs>
 
-270 cp
-
-#include <fcode/evaluator.fs>
-
 2e0 cp
 
 #include <quiesce.fs>
diff --git a/slof/fs/fbuffer.fs b/slof/fs/fbuffer.fs
index 4704608..bfe60eb 100644
--- a/slof/fs/fbuffer.fs
+++ b/slof/fs/fbuffer.fs
@@ -205,60 +205,6 @@  CREATE bitmap-buffer 400 4 * allot
 	screen-width screen-depth * to screen-line-bytes
 ;
 
-
-\ Install display related FCODE evaluator tokens
-: fb8-set-tokens  ( -- )
-	['] is-install        0 11C set-token
-	['] is-remove         0 11D set-token
-	['] is-selftest       0 11E set-token
-
-	['] #lines            0 150 set-token
-	['] #columns          0 151 set-token
-	['] line#             0 152 set-token
-	['] column#           0 153 set-token
-	['] inverse?          0 154 set-token
-	['] inverse-screen?   0 155 set-token
-	['] draw-character    0 157 set-token
-	['] reset-screen      0 158 set-token
-	['] toggle-cursor     0 159 set-token
-	['] erase-screen      0 15A set-token
-	['] blink-screen      0 15B set-token
-	['] invert-screen     0 15C set-token
-	['] insert-characters 0 15D set-token
-	['] delete-characters 0 15E set-token
-	['] insert-lines      0 15F set-token
-	['] delete-lines      0 160 set-token
-	['] draw-logo         0 161 set-token
-	['] frame-buffer-adr  0 162 set-token
-	['] screen-height     0 163 set-token
-	['] screen-width      0 164 set-token
-	['] window-top        0 165 set-token
-	['] window-left       0 166 set-token
-	\ ['] foreground-color  0 168 set-token  \ 16-color extension - n/a
-	\ ['] background-color  0 169 set-token  \ 16-color extension - n/a
-	['] default-font      0 16A set-token
-	['] set-font          0 16B set-token
-	['] char-height       0 16C set-token
-	['] char-width        0 16D set-token
-	['] >font             0 16E set-token
-	['] fontbytes         0 16F set-token
-
-	['] fb8-draw-character    0 180 set-token
-	['] fb8-reset-screen      0 181 set-token
-	['] fb8-toggle-cursor     0 182 set-token
-	['] fb8-erase-screen      0 183 set-token
-	['] fb8-blink-screen      0 184 set-token
-	['] fb8-invert-screen     0 185 set-token
-	['] fb8-insert-characters 0 186 set-token
-	['] fb8-delete-characters 0 187 set-token
-	['] fb8-insert-lines      0 188 set-token
-	['] fb8-delete-lines      0 189 set-token
-	['] fb8-draw-logo         0 18A set-token
-	['] fb8-install           0 18B set-token
-;
-fb8-set-tokens
-
-
 \ \\\\\\\\\\\\ Debug Stuff \\\\\\\\\\\\\\\\
 
 : fb8-dump-bitmap cr char-height 0 ?do char-width 0 ?do dup c@ if ." @" else ." ." then 1+ loop cr loop drop ;
diff --git a/slof/fs/fcode/tokens.fs b/slof/fs/fcode/tokens.fs
index 3efc17e..9e6f6bd 100644
--- a/slof/fs/fcode/tokens.fs
+++ b/slof/fs/fcode/tokens.fs
@@ -297,9 +297,9 @@  reset-token-table
 ' model             0 119 set-token
 ' device-type       0 11A set-token
 ' parse-2int        0 11B set-token
-\ ' is-install      0 11C set-token    \ Will be set by framebuffer code
-\ ' is-remove       0 11D set-token    \ Will be set by framebuffer code
-\ ' is-selftest     0 11E set-token    \ Will be set by framebuffer code
+' is-install        0 11C set-token    \ for framebuffer code
+' is-remove         0 11D set-token    \ for framebuffer code
+' is-selftest       0 11E set-token    \ for framebuffer code
 ' new-device        0 11F set-token
 ' diagnostic-mode?  0 120 set-token
 ' display-status    0 121 set-token    \ Maybe obsolete
@@ -321,56 +321,56 @@  reset-token-table
 
 \ Tokens 0x132 to 0x14f are reserved
 
-\ The following tokens will be set by the framebuffer code:
-\ ' #lines            0 150 set-token
-\ ' #columns          0 151 set-token
-\ ' line#             0 152 set-token
-\ ' column#           0 153 set-token
-\ ' inverse?          0 154 set-token
-\ ' inverse-screen?   0 155 set-token
+\ The following tokens are for the framebuffer code:
+' #lines            0 150 set-token
+' #columns          0 151 set-token
+' line#             0 152 set-token
+' column#           0 153 set-token
+' inverse?          0 154 set-token
+' inverse-screen?   0 155 set-token
 \ ' frame-buffer-busy 0 156 set-token  \ Historical, not supported
-\ ' draw-character    0 157 set-token
-\ ' reset-screen      0 158 set-token
-\ ' toggle-cursor     0 159 set-token
-\ ' erase-screen      0 15A set-token
-\ ' blink-screen      0 15B set-token
-\ ' invert-screen     0 15C set-token
-\ ' insert-characters 0 15D set-token
-\ ' delete-characters 0 15E set-token
-\ ' insert-lines      0 15F set-token
-\ ' delete-lines      0 160 set-token
-\ ' draw-logo         0 161 set-token
-\ ' frame-buffer-adr  0 162 set-token
-\ ' screen-height     0 163 set-token
-\ ' screen-width      0 164 set-token
-\ ' window-top        0 165 set-token
-\ ' window-left       0 166 set-token
+' draw-character    0 157 set-token
+' reset-screen      0 158 set-token
+' toggle-cursor     0 159 set-token
+' erase-screen      0 15A set-token
+' blink-screen      0 15B set-token
+' invert-screen     0 15C set-token
+' insert-characters 0 15D set-token
+' delete-characters 0 15E set-token
+' insert-lines      0 15F set-token
+' delete-lines      0 160 set-token
+' draw-logo         0 161 set-token
+' frame-buffer-adr  0 162 set-token
+' screen-height     0 163 set-token
+' screen-width      0 164 set-token
+' window-top        0 165 set-token
+' window-left       0 166 set-token
 \ '                   0 167 set-token  \ Reserved
 \ ' foreground-color  0 168 set-token  \ From 16-color recommended practice
 \ ' background-color  0 169 set-token  \ From 16-color recommended practice
-\ ' default-font      0 16A set-token
-\ ' set-font          0 16B set-token
-\ ' char-height       0 16C set-token
-\ ' char-width        0 16D set-token
-\ ' >font             0 16E set-token
-\ ' fontbytes         0 16F set-token
+' default-font      0 16A set-token
+' set-font          0 16B set-token
+' char-height       0 16C set-token
+' char-width        0 16D set-token
+' >font             0 16E set-token
+' fontbytes         0 16F set-token
 
 \ Tokens 0x170 to 0x17C are obsolete fb1 functions
 \ Tokens 0x17D to 0x17F are reserved
 
-\ The following tokens will be set by the framebuffer code, too:
-\ ' fb8-draw-character 0 180 set-token
-\ ' fb8-reset-screen   0 181 set-token
-\ ' fb8-toggle-cursor  0 182 set-token
-\ ' fb8-erase-screen   0 183 set-token
-\ ' fb8-blink-screen   0 184 set-token
-\ ' fb8-invert-screen  0 185 set-token
-\ ' fb8-insert-characters 0 186 set-token
-\ ' fb8-delete-characters 0 187 set-token
-\ ' fb8-insert-lines   0 188 set-token
-\ ' fb8-delete-lines   0 189 set-token
-\ ' fb8-draw-logo      0 18A set-token
-\ ' fb8-install        0 18B set-token
+\ The following tokens are for the framebuffer code, too:
+' fb8-draw-character 0 180 set-token
+' fb8-reset-screen   0 181 set-token
+' fb8-toggle-cursor  0 182 set-token
+' fb8-erase-screen   0 183 set-token
+' fb8-blink-screen   0 184 set-token
+' fb8-invert-screen  0 185 set-token
+' fb8-insert-characters 0 186 set-token
+' fb8-delete-characters 0 187 set-token
+' fb8-insert-lines   0 188 set-token
+' fb8-delete-lines   0 189 set-token
+' fb8-draw-logo      0 18A set-token
+' fb8-install        0 18B set-token
 
 \ Tokens 0x18C to 0x18F are reserved
 \ Tokens 0x190 to 0x196 are obsolete VMEbus tokens