Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Master start ff earlier #5181

Merged
merged 2 commits into from
Jan 22, 2024

Conversation

jkonecny12
Copy link
Member

@jkonecny12 jkonecny12 commented Sep 15, 2023

The execution time between Gnome Initial Setup and Anaconda is quite big
we need to reduce it by moving FF execution sooner before Anaconda is
executed.

Time to show Firefox window based on my measurements on VM with 2GB RAM
and 4vCPUs:
Original execution time: 13 seconds
With this commit: less than 6 seconds

If we want to reduce this time even more we need to do that in Firefox
or Gnome Shell (Gnome Initial Setup environment) somehow.

TODO:

  • Tests
  • Add JSX support so we don't have just white screen
  • [ ] Add event loop also to original webui-desktop script execution code?
    • This part is a question because I think we could have an issue without any event loop running but so far there are no complains about that...
    • Let's do that in separate PR so we don't complicate things here. The code won't be used on Live anyway.
  • Make the code more robust to not crash on unexpected inputs

Resolves: rhbz#2236438

Copy link
Contributor

@VladimirSlavik VladimirSlavik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. Thank you!

@jkonecny12
Copy link
Member Author

/build-image --live

@github-actions
Copy link

Images built based on commit 99f06cd:

  • Live: failure

Download the images from the bottom of the job status page.

@jkonecny12
Copy link
Member Author

@KKoukiou I have to focus on other things, so if you want and have time please feel free to finish this PR. The backend solution should be ready we just need to found a way to avoid having white screen in FF before Anaconda backend is ready.

def _run_webui(self):
# FIXME: This part should be start event loop (could use the WatchProcesses class)
# FIXME: We probably want to move this to early execution similar to what we have on live
profile_name = FIREFOX_THEME_DEFAULT
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The live vs boot.iso need different firefox theming. This is not a desired change.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is just code move. This code should be started only on boot.iso not on live so the theme here is correct. The other theme is specified in liveinst script because there is a place where the script is started.

pyanaconda/ui/webui/__init__.py Show resolved Hide resolved
@KKoukiou
Copy link
Contributor

/build-image --live

@github-actions
Copy link

Images built based on commit 99f06cd:

  • Live: failure

Download the images from the bottom of the job status page.

@pep8speaks
Copy link

pep8speaks commented Oct 16, 2023

Hello @jkonecny12! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

Line 89:9: E303 too many blank lines (2)
Line 128:100: E501 line too long (106 > 99 characters)
Line 139:9: E303 too many blank lines (2)
Line 151:100: E501 line too long (106 > 99 characters)
Line 189:100: E501 line too long (100 > 99 characters)

Comment last updated at 2024-01-22 14:43:52 UTC

@KKoukiou
Copy link
Contributor

Here is a screencast on the current behavior:

AwesomeScreenshot-10_16_2023.7.10.28PM.mp4

@KKoukiou KKoukiou marked this pull request as ready for review October 16, 2023 17:15
@jkonecny12
Copy link
Member Author

Updated with a new solution of a flag file signalizing that backend is ready to be consumed after Anaconda is initialized.

Web UI code was removed because it's in different repository now. @KKoukiou could you please help me there and link a PR here? The Web UI needs to wait until /run/anaconda/backend_ready is created.

@jkonecny12
Copy link
Member Author

/kickstart-test --testtype smoke

@jkonecny12
Copy link
Member Author

/packit build

1 similar comment
@jkonecny12
Copy link
Member Author

/packit build

Copy link
Contributor

@poncovka poncovka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

pyanaconda/ui/webui/__init__.py Outdated Show resolved Hide resolved
pyanaconda/ui/webui/__init__.py Outdated Show resolved Hide resolved
@jkonecny12
Copy link
Member Author

Verified together with rhinstaller/anaconda-webui#132 and on a spin (KDE Live). Everything seems to be working fine with the current code.

@jkonecny12
Copy link
Member Author

/packit build

@jkonecny12
Copy link
Member Author

/kickstart-test --testtype smoke

@jkonecny12
Copy link
Member Author

Screecast
Kooha-2024-01-17-16-22-35.webm

Based on my measuring it seems to be faster almost by 50%.
Without this patch: 10 seconds
With this patch: 6 seconds

Copy link
Contributor

@poncovka poncovka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

# Force the X11 backend since sudo and wayland do not mix
export GDK_BACKEND=x11

if [ -x /usr/bin/udisks ]; then
/usr/bin/udisks --inhibit -- "$ANACONDA" "$@"
/usr/bin/udisks --inhibit -- start_anaconda "$@"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it necessary to do the additional set up while inhibiting some of the udisks operations? I am not sure.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on the testing it should be fine. Also we inhibit the udisks for the whole Anaconda run if I'm reading this correctly which included start of the Firefox even before.

@vojtechtrefny do you have some recommendation for this?

Copy link
Contributor

@vojtechtrefny vojtechtrefny Jan 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This must be a very ancient check -- /usr/bin/udisks doesn't exists in udisks2, the old udisks package is still available in Fedora, but nobody is using it anymore (udisks 2.0 has been released 11 years ago). I think this can be safely removed.

CC @tbzatek

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well the surrounding check evaluates to FALSE anyway and was for some time already. So this can be safely removed.

If you still want to keep the inhibit, you need to to that via D-Bus call to UDisks (2). Unless you want to suspend the system in the middle of installation, I wouldn't worry about that much.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

heh, ok. In that case let's remove it in separate PR because it doesn't seem related to this. Thanks for info.

And I don't think we need that inhibit, it seems it works fine now so we probably don't need that...

@jkonecny12
Copy link
Member Author

/packit build

Copy link
Contributor

@rvykydal rvykydal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

The execution time between Gnome Initial Setup and Anaconda is quite big
we need to reduce it by moving FF execution sooner before Anaconda is
executed.

Time to show Firefox window based on my measurements on VM with 2GB RAM
and 4vCPUs:
Original execution time: 13 seconds
With this commit: less than 6 seconds

If we want to reduce this time even more we need to do that in Firefox
or Gnome Shell (Gnome Initial Setup environment) somehow.

Resolves: rhbz#2236438
Currently Anaconda doesn't have DBus API to signalize that backend is
ready to be consumed (initialized). For this reason Web UI don't know
when it can access the Web UI.

For this reason let's introduce a new flag file which is a signal for
Web UI that loading screen could be closed and Anaconda is ready to be
used.

Resolves: rhbz#2236438
@jkonecny12
Copy link
Member Author

Rebased.

@jkonecny12
Copy link
Member Author

/kickstart-test --testtype smoke

@jkonecny12
Copy link
Member Author

Broken test is caused by COPR repository containing older a newer builds. 40.8 and 40.17 and trying to install both versions. Could be probably resolved by removing old builds.

@jkonecny12
Copy link
Member Author

I'm not able to resolve it because I don't have permission to remove the old builds. Merging anyway - not related to change.

@jkonecny12 jkonecny12 merged commit 3f68ee9 into rhinstaller:master Jan 22, 2024
13 of 14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

9 participants