UnBounded History growth can consume lots of memory about surf HOT 19 OPEN

There's no procedure in place at the moment. Unless @lxt2 has some other idea, I would think we should tag v1.0.1 and then merge your changes.

from surf.

This should work:

b := surf.NewBrowser()
b.HistoryJar().SetMax(1)

from surf.

The PR was merged into the master branch, but has not been released with a version tag...

Checkout master and you should be seeing the new Methods.

from surf.

Do you actually need the History tracked for that? I'd probably offer a NoopHistory implementation that just dropped history. I don't use the History stuff either.

from surf.

Yes, I guess that would be another way to approach this. In my situation I use Bookmarks but I never go back into History ether. But if you need history, and it grows too big, then you are in a bad place again.

I used the std lib container/list which is a doubly linked list. In my code I set the max to 10 or so and allow it to flow through the list. Not a lot of overhead to manage the pointers in the list and it keeps the History option open.

from surf.

Having a Browser.Close() method which frees resources sounds like a good idea.

from surf.

If you called Browser.Close(), would you expect to be able to use it again?

With the current code, simply allowing the Browser object to go out of scope effectively releases all resources to the GC to cleanup.

My issue came in when I was needing to use the Browser over many requests. So I added a method Browser.ClearHistory() to explicitly clear history at a given point. I also added a Browser.SetMaxHistory(max int) to limit history growth to a number of items. I left the default at unlimited as to not change the exiting behavior and possibly cause a BC issue, but it might be okay to have a sensible default here of 100 items or something like that.

from surf.

The two methods to clear and limit the history sound good to me too, though I would probably put those methods on the jar.History interface. (In which case they would be named Clear() and SetMax().)

from surf.

Those two methods do exist on jar.History, but the current Browser does not provide access to Browser.history. So you would have to do something like this:

b := surf.NewBrowser()
hist := jar.NewMemoryHistory()
hist.Max(100)
b.SetHistoryJar(hist)

Because this felt to painful, I added the corresponding methods to the Browser.

b := surf.NewBrowser()
b.SetMaxHistory(100)

Maybe it would just be better to make the History object accessible.

from surf.

Maybe it would just be better to make the History object accessible.

I think that approach would work best, and if we're doing that, we should make the other jar objects accessible as well.

from surf.

Okay. I will work on that.

from surf.

I removed the methods browser.SetMaxHistory() and browser.ClearHistory(). I also exposed 4 jar objects so they can be manipulated directly.

from surf.

Looks good, though the Browsable interface is missing the getter methods. Thanks for your effort in making these changes!

from surf.

Done. We now have the same methods as part of the Browsable interface.

from surf.

The last item that I would like to do is change the default history length to 100. What is the procedure to make this in a patch release?

from surf.

Is there a PR for this that I've missed?

from surf.

I just added a PR to change the default.

from surf.

I do not understand now... how would you limit history to 1 page now?

from surf.

This should work:

b := surf.NewBrowser()
b.HistoryJar().SetMax(1)

There no HistoryJar() method

from surf.