Ensure garbage collection is able to clean up after bokeh server sessions are destroyed #3106

philippjfr · 2018-10-26T14:21:07Z

Cleans up plots when a bokeh server session is destroyed allowing the plots and objects to be garbage collected.

Here's a screenshot of the output of memprof of a bokeh server session where I open consecutive sessions and then close them. As you can see after all the sessions are cleaned up memory usage returns to the 200 MB baseline, which is used to keep all the sys.modules in memory and run a bokeh server.

Before:

After:

Without bokeh/bokeh#8368 the garbage collection will not happen immediately and things may stick around for a while. So we won't quite see the behavior shown above until bokeh 1.0.1 is released with that fix.

Closes Memory leak with bokeh serve and datashader #2111

philippjfr · 2018-10-26T22:29:31Z

The graphs which shows the memory usage going back down to 200 MB were all done while I still had code in bokeh which would print any uncollected references to certain objects which itself seems to have triggered some garbage collection. Without that the memory profile is slightly worse, but in the graph below I opened 10 sessions consecutively, closed them, waited for a bit, and repeated it two more times. You can see after each of the sets of 10 sessions it returns to ~230 MB.

philippjfr · 2018-10-26T22:32:11Z

Also worth noting that there are almost certainly still minor memory leaks, e.g. custom options are not getting cleaned up. This PR only deals with the most important leaks, i.e. both Elements and Streams, which can both hold actual and potentially very large data.

philippjfr · 2018-10-28T13:21:57Z

holoviews/__init__.py

@@ -38,7 +38,7 @@
    import IPython                 # noqa (API import)
    from .ipython import notebook_extension
    extension = notebook_extension # noqa (name remapping)
-except ImportError as e:
+except ImportError:


Unrelated, but started raising flake errors all of a sudden.

jlstevens · 2018-10-29T13:21:49Z

holoviews/plotting/bokeh/plot.py

        self._document = doc
        if self.subplots:
            for plot in self.subplots.values():
                if plot is not None:
                    plot.document = doc

+    def _session_destroy(self, session_context):


So the only reason this method exists is to drop session_context before calling cleanup?

Correct, bokeh checks signatures.

jlstevens · 2018-10-29T13:23:26Z

holoviews/plotting/links.py

-
-    target = param.ClassSelector(class_=ViewableElement, doc="""
-        The target object of the link (optional).""")
-


Shame to lose these as parameters.

True, although it did give me the idea of generalizing links at the panel level.

jlstevens · 2018-10-29T13:25:14Z

holoviews/streams.py



    @source.setter
    def source(self, source):
-        if self._source:
-            source_list = self.registry[id(self._source)]
+        old_source = self._source() if self._source else None


I assume you can't use a property getter when writing a setter?

It would be fine, and if there was any more logic I'd have opted for using the getter.

Personally, I think using self.source is nicer than defining old_source. Doesn't need to hold up this PR though...

jlstevens · 2018-10-29T13:26:26Z

holoviews/streams.py

+    # Mapping from a source to a list of streams
+    # WeakKeyDictionary to allow garbage collection
+    # of unreferenced sources
+    registry = weakref.WeakKeyDictionary()


If this is a dictionary type, doesn't this assume the keys are hashable? Using id meant you could always be sure you could use it as a key.

Afaik, none of the object types that we support as sources (elements, DynamicMaps, overlays etc.) implement any custom __hash__ method which would interfere with them being used as keys. The reason I used the id originally was to avoid references to the source sticking around, which was actually counter-productive it turns out.

jlstevens · 2018-10-29T13:30:51Z

Looks good to me. Once question, is it worth considering what happens when you use del on these various objects? For instance, we might want to think about implementing __del__ although I'm not yet convinced that the extra code would be worth it for supporting something used so rarely (and implementing __del__ is not always recommended anyway).

philippjfr · 2018-10-29T13:36:58Z

Once question, is it worth considering what happens when you use del on these various objects?

I'm not quite sure whether that's even necessary and what the custom __del__ would do. With this PR it should be sufficient to call del obj and due to the weak references that should be sufficient.

jlstevens · 2018-10-29T13:38:22Z

I was thinking a custom __del__ could do more clean up properly, e.g close comms. Anyway, it was just a thought and even if we decide it is a good idea, doesn't need to go into this PR.

philippjfr · 2018-10-29T13:40:24Z

I was thinking a custom del could do more clean up properly, e.g close comms

The comms are associated with plots/widgets, not with an element directly so I don't think that helps. It may end up being helpful for cleaning up custom option trees though.

jlstevens · 2018-10-29T13:44:23Z

What I had in mind was calling del on a BokehPlot instance. Anyway, it isn't important here. Merging.

…ions are destroyed (#3106)

github-actions · 2024-10-24T22:26:16Z

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

Ensure cleanup on session destroy

9d09241

philippjfr added the tag: backend: bokeh label Oct 26, 2018

philippjfr added 2 commits October 26, 2018 22:08

Fixed memory leak with weakrefs

0a79749

Added comments

49f5725

philippjfr changed the title ~~Ensure cleanup on bokeh server session destroy~~ Ensure garbage collections is able to clean up after bokeh server sessions are destroyed Oct 26, 2018

philippjfr changed the title ~~Ensure garbage collections is able to clean up after bokeh server sessions are destroyed~~ Ensure garbage collection is able to clean up after bokeh server sessions are destroyed Oct 26, 2018

philippjfr added this to the v1.10.8 milestone Oct 26, 2018

philippjfr added 4 commits October 27, 2018 01:37

Store Links in WeakKeyDictionary

4613030

Fixed flakes

1437b35

Fixed minor issues

6967487

Fixed flake

640284d

philippjfr commented Oct 28, 2018

View reviewed changes

jlstevens reviewed Oct 29, 2018

View reviewed changes

jlstevens merged commit 98b5e96 into master Oct 29, 2018

philippjfr added a commit that referenced this pull request Oct 29, 2018

Ensure garbage collection is able to clean up after bokeh server sess…

b067ba1

…ions are destroyed (#3106)

philippjfr added a commit that referenced this pull request Oct 29, 2018

Ensure garbage collection is able to clean up after bokeh server sess…

cbb07e1

…ions are destroyed (#3106)

philippjfr deleted the server_session_cleanup branch November 12, 2018 18:00

github-actions bot locked as resolved and limited conversation to collaborators Oct 24, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ensure garbage collection is able to clean up after bokeh server sessions are destroyed #3106

Ensure garbage collection is able to clean up after bokeh server sessions are destroyed #3106

philippjfr commented Oct 26, 2018 •

edited

Loading

philippjfr commented Oct 26, 2018 •

edited

Loading

philippjfr commented Oct 26, 2018 •

edited

Loading

philippjfr Oct 28, 2018

jlstevens Oct 29, 2018

philippjfr Oct 29, 2018

jlstevens Oct 29, 2018

philippjfr Oct 29, 2018

jlstevens Oct 29, 2018

philippjfr Oct 29, 2018

jlstevens Oct 29, 2018

jlstevens Oct 29, 2018

philippjfr Oct 29, 2018

jlstevens commented Oct 29, 2018 •

edited

Loading

philippjfr commented Oct 29, 2018 •

edited

Loading

jlstevens commented Oct 29, 2018

philippjfr commented Oct 29, 2018

jlstevens commented Oct 29, 2018

github-actions bot commented Oct 24, 2024


		target = param.ClassSelector(class_=ViewableElement, doc="""
		The target object of the link (optional).""")

Ensure garbage collection is able to clean up after bokeh server sessions are destroyed #3106

Ensure garbage collection is able to clean up after bokeh server sessions are destroyed #3106

Conversation

philippjfr commented Oct 26, 2018 • edited Loading

philippjfr commented Oct 26, 2018 • edited Loading

philippjfr commented Oct 26, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jlstevens commented Oct 29, 2018 • edited Loading

philippjfr commented Oct 29, 2018 • edited Loading

jlstevens commented Oct 29, 2018

philippjfr commented Oct 29, 2018

jlstevens commented Oct 29, 2018

github-actions bot commented Oct 24, 2024

philippjfr commented Oct 26, 2018 •

edited

Loading

philippjfr commented Oct 26, 2018 •

edited

Loading

philippjfr commented Oct 26, 2018 •

edited

Loading

jlstevens commented Oct 29, 2018 •

edited

Loading

philippjfr commented Oct 29, 2018 •

edited

Loading