Skip to content

Commit

Permalink
VERY IMPORTANT PERFORMANCE UPDATE : Decoupled Views from ProfileListener
Browse files Browse the repository at this point in the history
There was a serious performance issue with large logs. The UI would hang
or even OOM.

One of the underlying causes was that every view (Flat, Tree,
FlameGraph) was a Listener, and they all would be updated when a new
Profile was selected.

In case of the FlameGraph View, the performance hit is extremely large,
and the UI would hang.

Now, the views have been decoupled. The ProfileContext containing an
ObjectProperty<Profile> is created first, and the different views will
bind or unbind their own ObjectProperty<Profile> to that when they are
shown or hidden.

As a result, the rendition for each view only takes place at the time
they are shown, and even then only if the Profile changes. When they are
hidden, they no longer react to new Profiles emitted by the back end
until such a time as they are shown again.
  • Loading branch information
rx committed Dec 30, 2016
1 parent ca05377 commit afea8f5
Show file tree
Hide file tree
Showing 11 changed files with 318 additions and 256 deletions.
Expand Up @@ -19,22 +19,23 @@
package com.insightfullogic.honest_profiler.ports.javafx.controller; package com.insightfullogic.honest_profiler.ports.javafx.controller;


import com.insightfullogic.honest_profiler.core.profiles.FlameGraph; import com.insightfullogic.honest_profiler.core.profiles.FlameGraph;
import com.insightfullogic.honest_profiler.core.profiles.FlameGraphListener;
import com.insightfullogic.honest_profiler.ports.javafx.view.FlameGraphCanvas; import com.insightfullogic.honest_profiler.ports.javafx.view.FlameGraphCanvas;


import javafx.fxml.FXML; import javafx.fxml.FXML;
import javafx.scene.layout.VBox; import javafx.scene.layout.VBox;


public class FlameViewController extends AbstractController implements FlameGraphListener public class FlameViewController extends ProfileViewController<FlameGraph>
{ {
@FXML @FXML
private VBox rootContainer; private VBox rootContainer;


private FlameGraphCanvas flameView; private FlameGraphCanvas flameView;


@FXML @FXML
public void initialize() protected void initialize()
{ {
super.initialize(profileContext -> profileContext.flameGraphProperty());

flameView = new FlameGraphCanvas(); flameView = new FlameGraphCanvas();
rootContainer.getChildren().add(flameView); rootContainer.getChildren().add(flameView);
} }
Expand Down Expand Up @@ -63,8 +64,13 @@ public void refreshFlameView()
} }


@Override @Override
public void accept(final FlameGraph flameGraph) protected void refresh(FlameGraph flameGraph)
{ {
if (flameGraph == null)
{
return;
}

flameView.accept(flameGraph); flameView.accept(flameGraph);
} }
} }
Expand Up @@ -37,7 +37,6 @@
import com.insightfullogic.honest_profiler.core.profiles.Profile; import com.insightfullogic.honest_profiler.core.profiles.Profile;
import com.insightfullogic.honest_profiler.ports.javafx.controller.filter.FilterDialogController; import com.insightfullogic.honest_profiler.ports.javafx.controller.filter.FilterDialogController;
import com.insightfullogic.honest_profiler.ports.javafx.model.ApplicationContext; import com.insightfullogic.honest_profiler.ports.javafx.model.ApplicationContext;
import com.insightfullogic.honest_profiler.ports.javafx.model.ProfileContext;
import com.insightfullogic.honest_profiler.ports.javafx.model.filter.FilterSpecification; import com.insightfullogic.honest_profiler.ports.javafx.model.filter.FilterSpecification;
import com.insightfullogic.honest_profiler.ports.javafx.util.DialogUtil; import com.insightfullogic.honest_profiler.ports.javafx.util.DialogUtil;
import com.insightfullogic.honest_profiler.ports.javafx.util.report.ReportUtil; import com.insightfullogic.honest_profiler.ports.javafx.util.report.ReportUtil;
Expand All @@ -60,7 +59,7 @@
import javafx.scene.control.cell.PropertyValueFactory; import javafx.scene.control.cell.PropertyValueFactory;
import javafx.scene.input.MouseEvent; import javafx.scene.input.MouseEvent;


public class FlatViewController extends AbstractController public class FlatViewController extends ProfileViewController<Profile>
{ {
@FXML @FXML
private Button filterButton; private Button filterButton;
Expand Down Expand Up @@ -91,13 +90,13 @@ public class FlatViewController extends AbstractController
private FilterDialogController filterDialogController; private FilterDialogController filterDialogController;
private ObjectProperty<FilterSpecification> filterSpec; private ObjectProperty<FilterSpecification> filterSpec;


private ProfileContext profileContext;

private ProfileFilter currentFilter; private ProfileFilter currentFilter;


@FXML @FXML
private void initialize() protected void initialize()
{ {
super.initialize(profileContext -> profileContext.profileProperty());

info(filterButton, "Specify filters restricting the visible entries"); info(filterButton, "Specify filters restricting the visible entries");
info(compareButton, "Click to select another open profile to compare this profile against"); info(compareButton, "Click to select another open profile to compare this profile against");
info(exportButton, "Export the visible entries to a CSV file"); info(exportButton, "Export the visible entries to a CSV file");
Expand Down Expand Up @@ -126,25 +125,16 @@ private void initialize()
@Override @Override
public void setApplicationContext(ApplicationContext applicationContext) public void setApplicationContext(ApplicationContext applicationContext)
{ {

super.setApplicationContext(applicationContext); super.setApplicationContext(applicationContext);
filterDialogController.setApplicationContext(appCtx()); filterDialogController.setApplicationContext(appCtx());
} }


public void setProfileContext(ProfileContext profileContext)
{
this.profileContext = profileContext;
profileContext.profileProperty()
.addListener((property, oldValue, newValue) -> refresh(newValue));
}

// Initialization Helper Methods // Initialization Helper Methods


private void initializeComparison() private void initializeComparison()
{ {
compareButton.setGraphic(viewFor(COMPARE_16)); compareButton.setGraphic(viewFor(COMPARE_16));
compareButton compareButton.setTooltip(new Tooltip("Compare this profile with another open profile"));
.setTooltip(new Tooltip("Compare this profile with another open profile"));
compareButton.setOnMousePressed(new EventHandler<MouseEvent>() compareButton.setOnMousePressed(new EventHandler<MouseEvent>()
{ {
@Override @Override
Expand Down Expand Up @@ -178,7 +168,7 @@ private void initializeFilter()
newValue == null || !newValue.isFiltering() ? viewFor(FUNNEL_16) newValue == null || !newValue.isFiltering() ? viewFor(FUNNEL_16)
: viewFor(FUNNEL_ACTIVE_16)); : viewFor(FUNNEL_ACTIVE_16));
currentFilter = new ProfileFilter(newValue.getFilters()); currentFilter = new ProfileFilter(newValue.getFilters());
refresh(profileContext.getProfile()); refresh(getTarget());
}); });


filterButton.setGraphic(viewFor(FUNNEL_16)); filterButton.setGraphic(viewFor(FUNNEL_16));
Expand Down Expand Up @@ -212,8 +202,14 @@ private void configureTimeShareColumn(TableColumn<FlatProfileEntry, Number> colu


// Refresh Methods // Refresh Methods


private void refresh(Profile profile) @Override
protected void refresh(Profile profile)
{ {
if (profile == null)
{
return;
}

Profile newProfile = profile.copy(); Profile newProfile = profile.copy();
currentFilter.accept(newProfile); currentFilter.accept(newProfile);


Expand All @@ -233,14 +229,14 @@ private void refreshContextMenu(ContextMenu menu)


profileNames.forEach(name -> profileNames.forEach(name ->
{ {
if (name.equals(profileContext.getName())) if (name.equals(prContext().getName()))
{ {
return; return;
} }


MenuItem item = new MenuItem(name); MenuItem item = new MenuItem(name);
item.setOnAction( item.setOnAction(
event -> appCtx().createDiffView(profileContext.getName(), name)); event -> appCtx().createDiffView(prContext().getName(), name));
menu.getItems().add(item); menu.getItems().add(item);
}); });
} }
Expand Down

This file was deleted.

Expand Up @@ -18,26 +18,20 @@
**/ **/
package com.insightfullogic.honest_profiler.ports.javafx.controller; package com.insightfullogic.honest_profiler.ports.javafx.controller;


import static com.insightfullogic.honest_profiler.ports.javafx.ViewType.FLAME;
import static com.insightfullogic.honest_profiler.ports.javafx.ViewType.FLAT; import static com.insightfullogic.honest_profiler.ports.javafx.ViewType.FLAT;
import static com.insightfullogic.honest_profiler.ports.javafx.util.ConversionUtil.getStringConverterForType; import static com.insightfullogic.honest_profiler.ports.javafx.util.ConversionUtil.getStringConverterForType;


import com.insightfullogic.honest_profiler.core.profiles.Profile;
import com.insightfullogic.honest_profiler.core.profiles.ProfileListener;
import com.insightfullogic.honest_profiler.ports.javafx.ViewType; import com.insightfullogic.honest_profiler.ports.javafx.ViewType;
import com.insightfullogic.honest_profiler.ports.javafx.model.ApplicationContext; import com.insightfullogic.honest_profiler.ports.javafx.model.ApplicationContext;
import com.insightfullogic.honest_profiler.ports.javafx.model.ProfileContext; import com.insightfullogic.honest_profiler.ports.javafx.model.ProfileContext;
import com.insightfullogic.honest_profiler.ports.javafx.model.task.InitializeProfileTask;


import javafx.concurrent.Task;
import javafx.fxml.FXML; import javafx.fxml.FXML;
import javafx.scene.Node; import javafx.scene.Node;
import javafx.scene.control.ChoiceBox; import javafx.scene.control.ChoiceBox;
import javafx.scene.control.Label; import javafx.scene.control.Label;
import javafx.scene.layout.AnchorPane; import javafx.scene.layout.AnchorPane;


public class ProfileRootController extends AbstractController public class ProfileRootController extends AbstractController
implements ProfileListener, ProfileController
{ {
@FXML @FXML
private ChoiceBox<ViewType> viewChoice; private ChoiceBox<ViewType> viewChoice;
Expand All @@ -52,27 +46,13 @@ public class ProfileRootController extends AbstractController
@FXML @FXML
private FlameViewController flameController; private FlameViewController flameController;


private ProfileContext profileContext;

@FXML @FXML
public void initialize() public void initialize()
{ {
info( info(
viewChoice, viewChoice,
"Select the View : Flat View lists all methods as a list; Tree View shows the stack trees per thread; Flame View shows the Flame Graph"); "Select the View : Flat View lists all methods as a list; Tree View shows the stack trees per thread; Flame View shows the Flame Graph");
info(traceCount, "Shows the number of samples in the profile"); info(traceCount, "Shows the number of samples in the profile");

profileContext = new ProfileContext();
profileContext.addListeners(this);

flatController.setProfileContext(profileContext);
treeController.setProfileContext(profileContext);

viewChoice.getSelectionModel().selectedItemProperty()
.addListener((property, oldValue, newValue) -> show(newValue));
viewChoice.setConverter(getStringConverterForType(ViewType.class));
viewChoice.getItems().addAll(ViewType.values());
viewChoice.getSelectionModel().select(FLAT);
} }


// Instance Accessors // Instance Accessors
Expand All @@ -86,28 +66,22 @@ public void setApplicationContext(ApplicationContext applicationContext)
flameController.setApplicationContext(applicationContext); flameController.setApplicationContext(applicationContext);
} }


@Override public void setProfileContext(ProfileContext profileContext)
public ProfileContext getProfileContext()
{
return profileContext;
}

// ProfileListener Implementation

/**
* Not threadsafe: must be run on JavaFx thread.
*/
@Override
public void accept(Profile profile)
{ {
traceCount.setText(profile.getTraceCount() + " samples"); flatController.setProfileContext(profileContext);
} treeController.setProfileContext(profileContext);
flameController.setProfileContext(profileContext);


// Profiling startup traceCount.setText(profileContext.getProfile().getTraceCount() + " samples");
profileContext.profileProperty().addListener(
(property, oldValue, newValue) -> traceCount
.setText(newValue == null ? null : newValue.getTraceCount() + " samples"));


public Task<Void> getProfileInitializationTask(Object source, boolean live) viewChoice.setConverter(getStringConverterForType(ViewType.class));
{ viewChoice.getItems().addAll(ViewType.values());
return new InitializeProfileTask(appCtx(), profileContext, source, live, flameController); viewChoice.getSelectionModel().selectedItemProperty()
.addListener((property, oldValue, newValue) -> show(newValue));
viewChoice.getSelectionModel().select(FLAT);
} }


// View Switch // View Switch
Expand All @@ -121,9 +95,25 @@ private void show(ViewType viewType)
child.setVisible(viewType.ordinal() == i); child.setVisible(viewType.ordinal() == i);
} }


if (viewType == FLAME) switch (viewType)
{ {
flameController.refreshFlameView(); case FLAT:
treeController.deactivate();
flameController.deactivate();
flatController.activate();
break;
case TREE:
flatController.deactivate();
flameController.deactivate();
treeController.activate();
break;
case FLAME:
flameController.activate();
flameController.refreshFlameView();
flatController.deactivate();
treeController.deactivate();
break;
default:
} }
} }
} }
@@ -0,0 +1,54 @@
package com.insightfullogic.honest_profiler.ports.javafx.controller;

import java.util.function.Function;

import com.insightfullogic.honest_profiler.ports.javafx.model.ProfileContext;

import javafx.beans.property.ObjectProperty;
import javafx.beans.property.SimpleObjectProperty;
import javafx.beans.value.ObservableValue;

public abstract class ProfileViewController<T> extends AbstractController
{

private ProfileContext profileContext;

private ObjectProperty<T> target;
private Function<ProfileContext, ObservableValue<T>> targetExtractor;

protected void initialize(Function<ProfileContext, ObservableValue<T>> targetExtractor)
{
this.targetExtractor = targetExtractor;
target = new SimpleObjectProperty<>();
target.addListener((property, oldValue, newValue) -> refresh(newValue));
}

protected ProfileContext prContext()
{
return profileContext;
}

public void setProfileContext(ProfileContext profileContext)
{
this.profileContext = profileContext;
}

protected T getTarget()
{
return target.get();
}

// Activation

public void activate()
{
target.bind(targetExtractor.apply(profileContext));
}

public void deactivate()
{
target.unbind();
}

protected abstract void refresh(T target);
}

0 comments on commit afea8f5

Please sign in to comment.