-
-
Notifications
You must be signed in to change notification settings - Fork 404
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
Various fixes for bokeh static_source optimization #1413
Changes from 2 commits
8c20c28
bdc69a7
a203695
419d897
b2f7592
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,6 +18,7 @@ | |
from .options import Store, StoreOptions | ||
from .pprint import PrettyPrinter | ||
|
||
obj_id = id | ||
# Alias parameter support for pickle loading | ||
|
||
ALIASES = {'key_dimensions': 'kdims', 'value_dimensions': 'vdims', | ||
|
@@ -479,7 +480,7 @@ class LabelledData(param.Parameterized): | |
|
||
_deep_indexable = False | ||
|
||
def __init__(self, data, id=None, **params): | ||
def __init__(self, data, id=None, plot_id=None, **params): | ||
""" | ||
All LabelledData subclasses must supply data to the | ||
constructor, which will be held on the .data attribute. | ||
|
@@ -488,6 +489,7 @@ def __init__(self, data, id=None, **params): | |
""" | ||
self.data = data | ||
self.id = id | ||
self._plot_id = plot_id or obj_id(self) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just use |
||
if isinstance(params.get('label',None), tuple): | ||
(alias, long_name) = params['label'] | ||
label_sanitizer.add_aliases(**{alias:long_name}) | ||
|
@@ -531,6 +533,7 @@ def clone(self, data=None, shared_data=True, new_type=None, *args, **overrides): | |
|
||
if data is None and shared_data: | ||
data = self.data | ||
settings['plot_id'] = self._plot_id | ||
# Apply name mangling for __ attribute | ||
pos_args = getattr(self, '_' + type(self).__name__ + '__pos_params', []) | ||
return clone_type(data, *args, **{k:v for k,v in settings.items() | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -804,15 +804,16 @@ def update_frame(self, key, ranges=None, plot=None, element=None, empty=False): | |
# Cache frame object id to skip updating data if unchanged | ||
previous_id = self.handles.get('previous_id', None) | ||
if self.batched: | ||
current_id = sum(element.traverse(lambda x: id(x.data), [Element])) | ||
current_id = sum(element.traverse(lambda x: x._plot_id, [Element])) | ||
else: | ||
current_id = id(element.data) | ||
current_id = element._plot_id | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it no longer taking into account the |
||
self.handles['previous_id'] = current_id | ||
self.static_source = self.dynamic and (current_id == previous_id) | ||
self.static_source = (self.dynamic and (current_id == previous_id)) | ||
if self.batched: | ||
data, mapping = self.get_batched_data(element, ranges, empty) | ||
else: | ||
data, mapping = self.get_data(element, ranges, empty) | ||
|
||
if not self.static_source: | ||
self._update_datasource(source, data) | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -147,7 +147,7 @@ def sync_sources(self): | |
from the same object. | ||
""" | ||
get_sources = lambda x: (id(x.current_frame.data), x) | ||
filter_fn = lambda x: (x.shared_datasource and x.current_frame and | ||
filter_fn = lambda x: (x.shared_datasource and x.current_frame is not None and | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just to confirm: is this a a fix unrelated to the optimization itself? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Seems so, not sure why I pushed it here, but it is an important fix. |
||
not isinstance(x.current_frame.data, np.ndarray) | ||
and 'source' in x.handles) | ||
data_sources = self.traverse(get_sources, [filter_fn]) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -73,11 +73,12 @@ def current_handles(self): | |
if self.static and not self.dynamic: | ||
return handles | ||
|
||
|
||
element = self.current_frame | ||
previous_id = self.handles.get('previous_id', None) | ||
current_id = id(self.current_frame.data) if self.current_frame else None | ||
current_id = None if self.current_frame is None else id(element)+id(element.data) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Summing To check the identity of both the element and the data I would use the tuple |
||
for handle in self._update_handles: | ||
if (handle == 'source' and self.dynamic and | ||
current_id == previous_id): | ||
if (handle == 'source' and self.dynamic and current_id == previous_id): | ||
continue | ||
if handle in self.handles: | ||
handles.append(self.handles[handle]) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we ever actually set the
id
via the constructor?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, probably for clone. Nevermind my question!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, clone does all the time.