[AboutSystem] The system information replicant's height keeps growing

[bipolar]

This is on hrev56471, 32 bits.

After placing the new AboutSystem's "System Info" replicant on the Desktop... it keeps growing in height (around 2/3 "lines of text" every second), until all the text disappears through the top of the screen (the bottom with the replicant handler remains in place).

Quite a funny bug, actually :-D

[nephele]

is this the same as https://dev.haiku-os.org/ticket/17952 ?

[bipolar]

No. That ticket is about the height of the AboutSystem window itself (being a tad too tall by default, I notice that too).

This one is about the height of the replicant after you place it on the Desktop, that keeps growing, and growing...

[nephele]

ah okay, I can't see anything in the video (its just black for me) on vlc on iOS, but it is likely easily reproducible the I'd assume.

[bipolar]

The video was captured with BeScreenCapture (300x600, mp4, 10 fps, to keep it small). Plays well on Haiku and on Windows at least.

Regarding the bug... yeah, happens every time I place the replicant on the Desktop :-)

[Starcrasher]

On linux, lot of players are not playing it but mpv works. It seems to happen only on 32 bits. I have the replicant on my 64 bits VM for few days and it is stable.

[bipolar]

Replying to Starcrasher:

It seems to happen only on 32 bits.

Confirmed, yes.

I updated my 64 bit install to hrev56473, and there the replicant doesn't changes its size.

Rebooted into hrev56471 32 bits: the replicant still grows in height.

[bipolar]

Changed container from MPEG (BeScreenCapture settings) to MP4V, hopefully making playback easier.

[bipolar]

Adding what I could find so far.

The problem manifests on SysInfoView::Pulse().

Right after the line 911:

	float difference = newHeight - fCachedMinHeight;

I've added this quick debugging code:

	char buff[200];
	sprintf(buff, "AboutSystem(%s): newHeight(%.2f) - fCachedMinHeight(%.2f) = %.2f\n",
			fIsReplicant ? "Replicant": "App",
			newHeight, fCachedMinHeight, difference);
	syslog(1, buff);

After opening AboutSystem, and drag-and-droping the replicant to the desktop, I'm getting the following on the syslog file:

USER: AboutSystem(App): newHeight(261.00) - fCachedMinHeight(261.00) = 0.00
Last message repeated 27 times
USER: AboutSystem(Replicant): newHeight(26941892608.00) - fCachedMinHeight(26941892608.00) = 18.00
USER: AboutSystem(App): newHeight(261.00) - fCachedMinHeight(261.00) = 0.00
USER: AboutSystem(Replicant): newHeight(26941892608.00) - fCachedMinHeight(26941892608.00) = 18.00
USER: AboutSystem(App): newHeight(261.00) - fCachedMinHeight(261.00) = 0.00
USER: AboutSystem(Replicant): newHeight(26941892608.00) - fCachedMinHeight(26941892608.00) = 18.00
USER: AboutSystem(App): newHeight(261.00) - fCachedMinHeight(261.00) = 0.00

And the pattern keeps repeating until I close the app and remove the replicant.

Some questions:

• Regarding the bug: Why the change from 261.0 to 26941892608.00?

• Regarding my debug line: Why I get 26941892608.00 for both "newHeight" and "fCachedMinHeight" (even when "difference" == 18?).

And regarding the original code:

• What is the use case of changing SysInfoView's height on Pulse() if it sits on the desktop?

When is that expected to be necessary? To accomodate a longer uptime string eventually?

Note: I had to disable the system's AboutSystem from the boot menu to be able to get this far, as otherwise the replicant that gets created is the one from the system package, and not the one from the freshly compiled and executed AbotSystem binary. Quite counter-intuitive behaviour, that took me too much time to figure out.

[bipolar]

I think gcc2 is miscompiling the newer code...

If in SysInfoView::Pulse(), I split the line:

	float newHeight = fCachedBaseHeight + _UptimeHeight();

by introducing a temp variable, like this:

	float newHeight = fCachedBaseHeight;
	float _temp = _UptimeHeight();
	newHeight += _temp;

As long as I use that _temp variable (by using it on a syslog() call, for example), the bug disappears, and the replicant stops to grow tall.

When I comment out that syslog() call, the bug reappears.

So, with that in mind, I reverted all my changes, and tried just using different OPTIM values on the Jamfile (-O1, -Os, -O0).

Only -O0 makes the bug disappears.

[bipolar]

Proposed change: ​https://review.haiku-os.org/c/haiku/+/5723

[pulkomandy]

This isn't a very convincing fix. Why the extremely strange value of 26941892608.00? Clearly something is wrong there.

It just happens that with values so large, the substraction may not have enough precision and ends up rounding to 0 with optimizations disabled, but that's just being lucky.

So, why is the value so large in the first place?

[bipolar]

Replying to pulkomandy:

This isn't a very convincing fix. Why the extremely strange value of 26941892608.00? Clearly something is wrong there.

Totally agree. It was intended more like start talking point...

...And a workaround out of frustration after hitting the wall of my limited debugging skills, and wanting to have it not move around on the Desktop while working on #17957.

It just happens that with values so large, the substraction may not have enough precision and ends up rounding to 0 with optimizations disabled, but that's just being lucky.

So, why is the value so large in the first place?

I wish I knew how to debug it further.

I can only confirm that (after reapplying that syslog() call, and building with -O0) those big values are still there, but only for the replicant instance. That's for x86-32.

---

I now did the same for x86-64... I'm using there a larger font (12 vs 14), so the number are not equal, but there's still a hugh difference between the "app" values vs the "replicant" values:

With -O0 or -Os:

USER: AboutSystem(App): newHeight(304.00) - fCachedMinHeight(304.00) = 0.00
USER: AboutSystem(Replicant): newHeight(18681354240.00) - fCachedMinHeight(18681354240.00) = 0.00

With jam's default optimizations:

USER: AboutSystem(App): newHeight(304.00) - fCachedMinHeight(304.00) = 0.00
USER: AboutSystem(Replicant): newHeight(18681360384.00) - fCachedMinHeight(18681360384.00) = 0.00

Why that changes with different optimizations?

[pulkomandy]

My guess is that the initial value for this float number is not meant to be a float value at all. Either it is uninitialized, or it is some constant used by the layout system. So, if you optimize things away, you can keep it uninitialized or invalid. But if you don't optimize, the code will actually load it into a FPU register, and that always automatically does some "normalization" on it. The normalization ends up making the two values identical, and the substraction is then 0.

[jscipione]

The intention of the code is to grow the replicant up when Time Running (aka uptime) wraps to the next line on Pulse, but it shouldn’t grow every pulse… I did do some caching if variables to try and minimize the amount of code that gets run each Pulse so maybe my cached variables are getting optimized away.

[bipolar]

Replying to jscipione:

Thanks for the info!

Regarding the bug that manifest on 32 bits:

SysInfoView::_BaseHeight() returns the big values for the replicant instance, but normal ones for the non-replicant one.

This is due to SysInfoView's int32 members fLabelCount and fSubtextCount being used in _BaseHeight() calculations, while having values assigned in SysInfoView::SysInfoView(), but being left uninitialized in SysInfoView::SysInfoView(BArchive*).

Adding:

    fLabelCount = 5;
    fSubtextCount = 7;

To SysInfoView::SysInfoView(BArchive*) fixes the issue. Not sure that's the cleanest way of doing it, though...

Suggestions?

[jscipione]

That makes sense actually as the fLabels and fSubtexts arrays were jettisoned just before the code was included and I never actually got to test the code with that late change by another developer. It’s probably safe to set those variables to 5 and 7 again in the archive constructor so that they match what was they were in the normal constructor. I was trying to avoid hard-coding the variable count so if you want to go the extra mile you could extract the counts from the layout instead but that would take a bit more work to do any maybe it’s not worth it.

[bipolar]

Replying to jscipione:

Got it. Thanks a lot. I'll see what I can manage to do that with the little skills I have, and before I get distracted by other things :-)

At the very least the hard-coded version will be better than "just use OPTIM = -O0". :-D

[korli]

fixed in hrev56525