search

SDL-Hercules-390 / hyperion

The SDL Hercules 4.x Hyperion version of the System/370, ESA/390, and z/Architecture Emulator
Other
246 stars 92 forks source link

Rubato Thread cpu usage #682

Closed dandole closed 2 months ago

dandole commented 2 months ago

In the rubato_thread (timer.c), I have noticed (relatively) high cpu usage on linux based hosts, when zArch guest is stopped or idle.

The comment states: https://github.com/SDL-Hercules-390/hyperion/blob/4a5d02fd2a9fb51dde641888472fa943d1a241b2/timer.c#L409-L418

This line of code (the throttle): https://github.com/SDL-Hercules-390/hyperion/blob/4a5d02fd2a9fb51dde641888472fa943d1a241b2/timer.c#L421

can result in values that are low or even negative.

This line of code is the Guard for too high or low of a value: https://github.com/SDL-Hercules-390/hyperion/blob/4a5d02fd2a9fb51dde641888472fa943d1a241b2/timer.c#L424

But MIN_TOD_UPDATE_USECS is set to 50, and not the minimum interval value set by TIMERINT by the user.

Shouldn't the minimum check be set using sysblk.timerint instead???   E.g.:

    MINMAX( new_timerint_usecs, sysblk.timerint, MAX_TOD_UPDATE_USECS );
Fish-Git commented 2 months ago

But MIN_TOD_UPDATE_USECS is set to 50, and not the minimum interval value set by TIMERINT by the user.

Shouldn't the minimum check be set using sysblk.timerint instead???   E.g.:

    MINMAX( new_timerint_usecs, sysblk.timerint, MAX_TOD_UPDATE_USECS );

Actually, no.

The user-specified sysblk.timerint value might not be within the valid TXF range. When TXF is enabled and active, the timerint value must always remain within the minimum/maximum TXF value, which is from 200 to 999999 usecs.

When TXF is not enabled/active, the minimum/maximum timerint value is 50-999999 usecs.

So while you are wrong, you DID find the bug that might explain your excessive cpu usage!

WELL DONE, Dan!   :)

The statement in question should actually be:

    MINMAX( new_timerint_usecs, MIN_TXF_TIMERINT, MAX_TOD_UPDATE_USECS );

(i.e. MIN_TXF_TIMERINT (which is 200 usecs), not MIN_TOD_UPDATE_USECS (which is 50).

I'll try to get that fixed for you right away.

Fish-Git commented 2 months ago

Fixed by commit f2de249875964d642fed9f5515e574375d81ae31.

Closing.