TRTE Performance Improvement #510

JamesWekel · 2022-10-08T20:19:42Z

Fish,

Let's try this pull request for TRTE performance improvement (third time lucky?).

I appreciate your review especially related to cross page requirements.

Before the change, the performance test reported:

         1,000,000 iterations of TRTE  took   4,496,345 microseconds
         1,000,000 iterations of TRTE  took   4,789,395 microseconds
         1,000,000 iterations of TRTE  took  17,574,283 microseconds
         1,000,000 iterations of TRTE  took  18,853,621 microseconds

and after the change:

          1,000,000 iterations of TRTE  took   1,039,821 microseconds
          1,000,000 iterations of TRTE  took   1,554,219 microseconds
          1,000,000 iterations of TRTE  took   2,657,194 microseconds
          1,000,000 iterations of TRTE  took   3,181,454 microseconds

The performance tests were run on I5-1135G7 processor.

The TRTE-02-performance tests the TRTE instruction with m3=12 where the FC table is 128K in length, FC is 2 bytes and an argument length of 2 bytes. This should be the worst performing TRTE instruction because of additional page boundary tests. Over three test runs, the average improvement ranged from 67 to 85%.

Thanks for your review. I'm sure that there is still something that I've missed.

Jim

Fish-Git

Not just no, but HELL no! :(

I'm sorry, James, but...

While it could certainly be argued that technically it shouldn't make any difference whether the integrity tests run in S/370 mode or z/Architecture mode (since the ultimate goal is, after all, to only test the proper functioning of the TRTE instruction itself), having an integrity test meant to test the proper functioning of a z/Architecture-only instruction run in a mode other than z/Architecture itself just does not sit well with me. It just doesn't seem right. I'm Sorry.

The changes necessary to have your two tests run in z/Arch mode instead of 370 mode are actually quite minor, so I'm going to have to insist that they be made before I can accept your pull request.

The actual TRTE instruction code in general2.c looks fine. I'm actually impressed by it in fact! The idea of pre-resolving the entire translation table to mainstor addresses beforehand is a rather brilliant one IMO. Well done! :)

Attached is a simple patch that illustrates the minimum changes needed to fix the problem with the two tests. As you can see the number of changes required are actually quite small:(*)

james-z.zip (*)

Soooo.... Once you make the above mentioned minor change to each of your two integrity tests, I would be very happy to accept your pull request.

But as it stands right now? Nope. Sorry! :(

(and I hope there's no hard feelings about all this!)

(*) NOTE: I did not bother including any cosmetic type changes in the attached patch (except in the .tst scripts). I'll take care of doing that myself after accepting your pull. The above patch only illustrates the minimum necessary changes that need to be made to allow your tests to run in z/Arch mode. I did not bother to fix any of the comments in your program for example. I'll leave that for you.

Fish-Git · 2022-10-09T20:44:57Z

P.S. There is also no need to close this pull request and create a new one. I'm sure GitHub has a way for you to re-submit your existing pull request again after making changes.

JamesWekel · 2022-10-10T02:19:24Z

Fish,

My background is S/370 and S/390, no ESA or z/Arch so that's where I went with the tests. Thank you for the patch to convert them to z/Arch. I totally understand your rational for the requirement. Learnt something new today!

I resubmitted new pull requests as I deleted my GitHub branch and recreated it after syncing to your 'develop' branch. Then I rebase my local branch on the new remote branch. As I wasn't sure what GitHub does with a pull request when the pulled branch is deleted, I thought it was safer to create a new pull request.

What standard font do you want for the PDF version of the test assemble listing? My standard coding font is 'Fira Code' so my PDF's are embedding 'Fira Code Medium:11'.

Jim

Fish-Git · 2022-10-10T03:36:40Z

What standard font do you want for the PDF version of the test assemble listing?

Whatever font you personally feel is a good one. Your choice.

Me? I prefer Consolas myself. It's slightly darker (thicker I guess) than old fashioned Courier or Courier New, so it looks good in a PDF listing IMHO (and its zero has slash through it too, to distinguish it from the letter capital "O"). Inconsolata is another nice one, which I believe was based on Consolas. Both I believe are embeddable, which IMO is important, as it ensures the PDF displays identically on all systems, even those that don't happen to have it installed.

But really, use whatever font you want.

My standard coding font is 'Fira Code' so my PDF's are embedding 'Fira Code Medium:11'.

Whatever! If you like it, use it! As I said, font preferences are typically quite personal. The font one person prefers someone else might hate, and vice-versa. So since whatever font you choose is going to be a real crap shoot anyway, I say go with whichever font you yourself personally prefer. Who knows? Someone who's never seen it before might well end up liking it themselves too! So I say go for it! :)

Fish-Git

Looks good to me! :)

Fish-Git · 2022-10-10T21:02:00Z

James,

How did you configure your SATK installation? I'm guessing you specified addr = 24 in your ASMA.cfg file. Yes?

The reason I ask is because your ASMA listings are formatted slightly differently from my ASMA listings. I suspect it's because I'm using addr = 31 in my ASMA.cfg file, whereas I'm guessing you're using addr = 24. Yes?

(sample snippet from mine):

ASMA Ver. 0.2.1              TRTE-01-basic (Test TRTE instructions)

  LOC        OBJECT CODE       ADDR1     ADDR2    STMT

00000000                      00000000  000001A0    44          ORG   TRTE1TST+X'1A0'
000001A0  00000001 80000000                         45          DC    X'0000000180000000'
000001A8  00000000 00000200                         46          DC    AD(BEGIN)


000001B0                      000001B0  000001D0    48          ORG   TRTE1TST+X'1D0'
000001D0  00020001 80000000                         49          DC    X'0002000180000000'
000001D8  00000000 0000DEAD                         50          DC    AD(X'DEAD')

(sample snippet from yours):

ASMA Ver. 0.2.1              TRTE-01-basic (Test TRTE instructions)

 LOC       OBJECT CODE      ADDR1   ADDR2   STMT

000000                      000000  0001A0    51          ORG   TRTE1TST+X'1A0'
0001A0  00000001 80000000                     52          DC    X'0000000180000000'
0001A8  00000000 00000200                     53          DC    AD(BEGIN)


0001B0                      0001B0  0001D0    55          ORG   TRTE1TST+X'1D0'
0001D0  00020001 80000000                     56          DC    X'0002000180000000'
0001D8  00000000 0000DEAD                     57          DC    AD(X'DEAD')

Notice the column alignment. They're different. Your LOC and ADDR1 and ADDR2 columns are only 6 hex digits wide (24 bits) whereas mine are 8 hex digits wide (31 bits).

Once again, while it could be argued that on the grand scale of things it probably makes little difference one way or the other which "addr" setting one chooses to configure their ASMA with, given that it's technically possible to define an EQU (equate) with a value larger than 16M-1, it would probably be safer IMHO to use addr = 31 instead.

For example: take a look at the following two listings:

addr = 24:

ASMA Ver. 0.2.1

 LOC       OBJECT CODE      ADDR1   ADDR2   STMT

                            0F4240  000001     1 MILLION EQU 1000000
                            9ACA00  000001     2 BILLION EQU 1000000000
                                               3
000000  000F4240                               4 AMILLION DC A(MILLION)
000004  3B9ACA00                               5 ABILLION DC A(BILLION)
                                               6
                                               7      END

addr = 31:

ASMA Ver. 0.2.1

  LOC        OBJECT CODE       ADDR1     ADDR2    STMT

                              000F4240  00000001     1 MILLION EQU 1000000
                              3B9ACA00  00000001     2 BILLION EQU 1000000000
                                                     3
00000000  000F4240                                   4 AMILLION DC A(MILLION)
00000004  3B9ACA00                                   5 ABILLION DC A(BILLION)
                                                     6
                                                     7      END

Notice the value that ASMA prints under the ADDR1 column. One is correct. The other one isn't. One is quite misleading. The other one isn't.

Therefore, given that most of our tests (not all, but certainly most) are designed for z/Arch (or at least S/390 or greater, usually), it would probably be best IMHO if you used an addr setting of 31 in your ASMA configuration instead of 24.

You don't have to of course! You can continue to use 24 if you want. It's your system after all! You do what you want! I'm just passing on friendly advice, that's all. Make of it what you will. It's no skin off my nose. <shrug>

P.S. What program are you using to convert your ASMA listings to PDF with? I installed the "Fira Code" font on my Windows 7 x64 system and my listings look a lot different from yours:

TRTE-01-basic.pdf (yours, as committed)
TRTE-01-basic.pdf (mine, using txt2pdf)

Since I'm using Windows (whereas it appears you're using Linux), I use my HercPrt product's txt2pdf inhouse tool to convert ASMA listings to PDFs (although I doubt which tool is used to perform the conversion makes any difference). What text-to-PDF tool are you using on your system? It's no big deal! I'm just curious, that's all. I'd simply like to understand why my listings look so much darker (and prettier IMHO!) than yours do.

Thanks.

P.P.S. I kind of like the Fira Code font! I might have to start using it myself! :)

Fish-Git · 2022-10-10T21:36:40Z

And just in case the two PDFs I uploaded look the same to you (perhaps because of the particular PDF Viewer you're using), here are some screen shots of what I'm seeing on my Windows system:

Fish-TRTE-01-Fira-Code-Medium:

James-TRTE-01-Fira-Code-Medium:

wrljet · 2022-10-10T21:38:39Z

That's with a fresh vs. a worn-out line printer ribbon. :-)

Fish-Git · 2022-10-10T23:29:59Z

That's with a fresh vs. a worn-out line printer ribbon. :-)

(Heh!) :)

JamesWekel · 2022-10-11T19:16:50Z

Fish wrote:

How did you configure your SATK installation? I'm guessing you specified addr = 24 in your ASMA.cfg file. Yes? The reason I ask is because your ASMA listings are formatted slightly differently from my ASMA listings. I suspect it's because I'm using addr = 31 in my ASMA.cfg file, whereas I'm guessing you're using addr = 24. Yes?

My SATK installation is just an unzip of a downloaded SATK GitHub master branch. No additional configuration. And I didn’t find any *.cfg files in the unzipped directory tree. I use just a modified asm script to update directories. For TRTE-01-basic.asm, the script executed: ${ASMA} -i ./$1.core -l $1-asm-$sfx.list $1.asm From the SATK documentation, the default is “the address size specified within the MSL for the target CPU.” With no default target specified, the default is -t 24. So, yep I’m defaulting to addr =24 for TRTE (I was using the s370 backport for TRTE). On CLCLE and TRE, I specified “-t s390”, so those should have 31 bit addresses. Still learning the environment. I’ll ensure that I have 31 bit addresses going forward.

P.S. What program are you using to convert your ASMA listings to PDF with? I installed the "Fira Code Medium" font on my Windows 7 x64 system and my listings look a lot different from yours: * TRTE-01-basic.pdf <https://github.com/SDL-Hercules-390/hyperion/files/9749209/TRTE-01-basic.pdf> (yours, as committed) * TRTE-01-basic.pdf <https://github.com/SDL-Hercules-390/hyperion/files/9749214/TRTE-01-basic.pdf> (mine, using txt2pdf) Since I'm using Windows (whereas it appears you're using Linux), I use my HercPrt product's txt2pdf inhouse tool to convert ASMA listings to PDFs (although I doubt which tool is used to perform the conversion makes any difference). What text-to-PDF tool are you using on your system? It's no big deal! I'm just curious, that's all. I'd simply like to understand why my listings look so much darker (and prettier IMHO!) than yours do.

I too am confused as to why they are so different. I’m using your HercPrt product’s txt2pdf to do the conversion. I download the listing to Windows 10, convert it and then upload the PDF back to Linux. Maybe my system is using the default weight (300 “Light”) rather the Medium weight (500)? Not sure why, so I’ll do some investigation. I also like the bolder, fresh printer ribbon look! Jim

Fish-Git · 2022-10-11T20:26:35Z

I’m using your HercPrt product’s txt2pdf to do the conversion. I download the listing to Windows 10, convert it and the upload the PDF back to Linux.

How did you install the Fira font on your Windows 10 system? I'm not familiar with Windows 10. I still use Windows 7, and all I did was download the "Fira_Code_v6.2.zip" file from https://github.com/tonsky/FiraCode, unzip it, and then double-click on the "FiraCode-Medium.ttf" file (inside the "ttf" folder), and Windows automatically installed the entire Fira font family of True Type fonts for me.

Hopefully Windows 10 hasn't changed TOO much, so hopefully you should be able to navigate to the Control Panel and display your Windows 10 system's Font directory. You should see a "Fira Code" icon there:

Double-clicking on that icon should display another window (folder) showing you all of the fonts that are installed for that entire font family:

(Fromm there you can double-click on whatever icon you want to see what the font actually looks like if you're curious)

Then of course when you run txt2pdf, you simply specify that font for the -f option, being sure to prefix the font name with a "+" sign to cause it to be embedded into the resulting PDF file:

txt2pdf -g -f "+Fira Code Medium:11" -i TRTE-01-basic.list
txt2pdf -g -f "+Fira Code Medium:11" -i TRTE-02-performance.list

That should work. If it doesn't, then I suspect something isn't right with your Windows 10 installation. txt2pdf simply creates a .PDF file (with the font embedded within it) but does not otherwise control how that font is displayed by whatever PDF Viewer you happen to use.

The ONLY thing I can possibly think of that MIGHT have some affect (although I have never personally tested this myself) is, if the listing file that txt2pdf is processing ends with only *Nix LF characters instead of the normal CR+LF (carriage-return + line-feed) line endings that Windows normally expects.

Maybe you can try running your listing file through unix2dos before running it through txt2pdf to see whether that makes any difference or not. If it doesn't (and I suspect it won't), then the only other explanation is your Windows 10 system. Either the Fira font isn't properly installed or something else weird is going on.

<shrug>

Fish-Git · 2022-10-11T20:29:56Z

p.s. You can still reply directly via GitHub (rather than respond via email, which is discouraged) by simply visiting the Pull Request's original URL:

TRTE Performance Improvement #510

JamesWekel · 2022-10-11T22:12:04Z

Fish,

Solved! It was my problem, as I download the Fira Code fonts from Google Font and installed the variable font .ttf file rather than the static .ttf files. Once I deleted the variable font and reinstalled the static files, the txt2pdf output PDF is using the Medium weight font. A bold, new printer ribbon.

Google Fonts has the Fira Code version 5. Thank you for the pointer to "Fira_Code_v6.2.zip". I've the newer version installed.

Jim

Fish-Git · 2022-10-11T22:32:22Z

Solved!
<snip details>

Great! Glad to hear it. Glad I could help. :)

JamesWekel added 6 commits October 7, 2022 08:03

TRTE Performance Improvement

86bdbd9

Performance Test - disable as default

9a410fc

Embed font in PDF

8707eb8

Fix perfromance test - missing instructions

28e05c9

Merge indentation changes

975acc1

Update TRTE tests

433670f

JamesWekel requested review from Fish-Git, ivan-w, Juergen-Git and Peter-J-Jansen as code owners October 8, 2022 20:19

Fish-Git requested changes Oct 9, 2022

View reviewed changes

Change tests to z/arch

19da9ce

Fish-Git approved these changes Oct 10, 2022

View reviewed changes

Fish-Git merged commit c5244cc into SDL-Hercules-390:develop Oct 10, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

TRTE Performance Improvement #510

TRTE Performance Improvement #510

JamesWekel commented Oct 8, 2022 •

edited by Fish-Git

Loading

Fish-Git left a comment •

edited

Loading

Fish-Git commented Oct 9, 2022

JamesWekel commented Oct 10, 2022 •

edited by Fish-Git

Loading

Fish-Git commented Oct 10, 2022

Fish-Git left a comment

Fish-Git commented Oct 10, 2022 •

edited

Loading

Fish-Git commented Oct 10, 2022 •

edited

Loading

wrljet commented Oct 10, 2022 •

edited by Fish-Git

Loading

Fish-Git commented Oct 10, 2022

JamesWekel commented Oct 11, 2022 via email •

edited by Fish-Git

Loading

Fish-Git commented Oct 11, 2022 •

edited

Loading

Fish-Git commented Oct 11, 2022

JamesWekel commented Oct 11, 2022 •

edited by Fish-Git

Loading

Fish-Git commented Oct 11, 2022 •

edited

Loading

TRTE Performance Improvement #510

TRTE Performance Improvement #510

Conversation

JamesWekel commented Oct 8, 2022 • edited by Fish-Git Loading

Fish-Git left a comment • edited Loading

Choose a reason for hiding this comment

Fish-Git commented Oct 9, 2022

JamesWekel commented Oct 10, 2022 • edited by Fish-Git Loading

Fish-Git commented Oct 10, 2022

Fish-Git left a comment

Choose a reason for hiding this comment

Fish-Git commented Oct 10, 2022 • edited Loading

Fish-Git commented Oct 10, 2022 • edited Loading

wrljet commented Oct 10, 2022 • edited by Fish-Git Loading

Fish-Git commented Oct 10, 2022

JamesWekel commented Oct 11, 2022 via email • edited by Fish-Git Loading

Fish-Git commented Oct 11, 2022 • edited Loading

Fish-Git commented Oct 11, 2022

JamesWekel commented Oct 11, 2022 • edited by Fish-Git Loading

Fish-Git commented Oct 11, 2022 • edited Loading

JamesWekel commented Oct 8, 2022 •

edited by Fish-Git

Loading

Fish-Git left a comment •

edited

Loading

JamesWekel commented Oct 10, 2022 •

edited by Fish-Git

Loading

Fish-Git commented Oct 10, 2022 •

edited

Loading

Fish-Git commented Oct 10, 2022 •

edited

Loading

wrljet commented Oct 10, 2022 •

edited by Fish-Git

Loading

JamesWekel commented Oct 11, 2022 via email •

edited by Fish-Git

Loading

Fish-Git commented Oct 11, 2022 •

edited

Loading

JamesWekel commented Oct 11, 2022 •

edited by Fish-Git

Loading

Fish-Git commented Oct 11, 2022 •

edited

Loading