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

Memory usage surge when selecting/appending MultilineEntry text #3426

Closed
2 tasks done
aztack opened this issue Nov 24, 2022 · 7 comments
Closed
2 tasks done

Memory usage surge when selecting/appending MultilineEntry text #3426

aztack opened this issue Nov 24, 2022 · 7 comments
Labels
bug Something isn't working

Comments

@aztack
Copy link

aztack commented Nov 24, 2022

Checklist

  • I have searched the issue tracker for open issues that relate to the same problem, before opening a new one.
  • This issue only relates to a single bug. I will open new issues for any other problems.

Describe the bug

My fyne App is just a node.exe launcher. It launcher node.exe and capture its output and write to a text entry.
Memory usage surge when selecting/appending MultilineEntry text as the following gif shows.
Memory usage keeps growing when the output of the node script is captured to the text entry. After about an hour of running, the memory usage peak at an astonishing 12G.

How to reproduce

  1. Create a multiline entry with a lot text content
  2. Keep Selecting text in the entry and monitor the memory usage of the app

Screenshots

20221124151652_rec_

image

Example code

	text := widget.NewMultiLineEntry()
	text.SetText("Press button 'Run' to Start")
	text.SetMinRowsVisible(16)

       var c *cmd.Cmd = cmd.NewCmdOptions(cmdOptions,  'node', newArgs...)
		go func() {
			complete := c.Status().Complete
			for !complete && c.Stdout != nil || c.Stderr != nil {
				complete = c.Status().Complete
				if !complete {
					if !runButton.Disabled() {
						runButton.Disable()
					}
					if stopButton.Disabled() {
						stopButton.Enable()
					}
				}
				select {
				case line, open := <-c.Stdout:
					// OK
					if !open {
						c.Stdout = nil
						continue
					}
					log.Println(line)
					text.SetText(text.Text + "\n" + line)
				case line, open := <-c.Stderr:
					// Error
					if !open {
						c.Stderr = nil
						continue
					}
					fmt.Fprintln(os.Stderr, line)
					text.SetText(text.Text + "\n" + line)
				}
			}
			stopButton.Disable()
			runButton.Enable()
		}()
		c.Start()
		a.Lifecycle().SetOnStopped(func() {
			log.Println("Stopped")
			c.Stop()
		})

Fyne version

2.2.3

Go compiler version

1.17

Operating system

macOS

Operating system version

12.5.1

Additional Information

No response

@aztack aztack added the unverified A bug that has been reported but not verified label Nov 24, 2022
@aztack
Copy link
Author

aztack commented Nov 24, 2022

maybe related: #2969 #3413

@andydotxyz
Copy link
Member

Thanks for this. It is very strange that selecting should do this, as everything should be a cache hit regarding the text rendering.
May not be duplicate of the issues you listed, so good to track this separately.

@andydotxyz andydotxyz added this to the Cragganmore Release (v2.3) milestone Nov 24, 2022
@andydotxyz
Copy link
Member

I can replicate this quite easily. The moving of selection is triggering refresh, which we should avoid as we don't have to refresh the whole widget.

This is probably separate to the linked issues which are really the source for the huge growth as the text rendering is clearly not clearing out old textures

@andydotxyz andydotxyz added bug Something isn't working and removed unverified A bug that has been reported but not verified labels Nov 26, 2022
andydotxyz added a commit to andydotxyz/fyne that referenced this issue Nov 26, 2022
There is an underlying issue where text refresh memory is not freed

but here we are able to avoid refresh which is always good.
Fixes fyne-io#3426
@aztack
Copy link
Author

aztack commented Nov 28, 2022

Nice work guys!
Is there any way I can use the fix in my project right away or in foreseeable future?

@andydotxyz
Copy link
Member

It has been landed on develop now, so you can test before release by running go get fyne.io/fyne/v2@develop

@aztack
Copy link
Author

aztack commented Nov 30, 2022

@andydotxyz I tried fyne.io/fyne/v2@develop. Selecting text no longer causes memory surge. This is great.
But a long text in a multiline entry still causes HUGE memory usage. Is there any suggestion to mitigate this issue?

@andydotxyz
Copy link
Member

Indeed, sorry for this. There is a separate issue that we are working on - texture cache hits with text are basically not firing so we keep rendering new content when anything changes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants