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.

Sign up for GitHub

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

[JENKINS-65821] Introducing some synchronisation mechanisms to prevent some race condition #153

Merged
merged 5 commits into from
Jun 7, 2021
Merged

[JENKINS-65821] Introducing some synchronisation mechanisms to prevent some race condition #153

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
61da1b2
Introducing some synchronization mechanisms to try improving performance
twasyl Jun 4, 2021
d13c9a6
Using a lock instead of synchronization
twasyl Jun 4, 2021
689f233
Synchronization at instance level as suggested in PR
twasyl Jun 7, 2021
8f5016c
Revert some unrelated changes
twasyl Jun 7, 2021
4f4aaa5
Revert some unrelated changes
twasyl Jun 7, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 7 additions & 10 deletions src/main/java/org/jenkinsci/plugins/workflow/graph/StandardGraphLookupView.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,20 +24,20 @@ public final class StandardGraphLookupView implements GraphLookupView, GraphList

static final String INCOMPLETE = "";

/** Map the blockStartNode to its endNode, to accellerate a range of operations */
/** Map the blockStartNode to its endNode, to accelerate a range of operations */
HashMap<String, String> blockStartToEnd = new HashMap<>();

/** Map a node to its nearest enclosing block */
HashMap<String, String> nearestEnclosingBlock = new HashMap<>();

public void clearCache() {
public synchronized void clearCache() {
blockStartToEnd.clear();
nearestEnclosingBlock.clear();
}

/** Update with a new node added to the flowgraph */
@Override
public void onNewHead(@Nonnull FlowNode newHead) {
public synchronized void onNewHead(@Nonnull FlowNode newHead) {
if (newHead instanceof BlockEndNode) {
blockStartToEnd.put(((BlockEndNode)newHead).getStartNode().getId(), newHead.getId());
String overallEnclosing = nearestEnclosingBlock.get(((BlockEndNode) newHead).getStartNode().getId());
Expand Down Expand Up @@ -87,7 +87,7 @@ public boolean isActive(@Nonnull FlowNode node) {
}

// Do a brute-force scan for the block end matching the start, caching info along the way for future use
BlockEndNode bruteForceScanForEnd(@Nonnull BlockStartNode start) {
synchronized BlockEndNode bruteForceScanForEnd(@Nonnull BlockStartNode start) {
DepthFirstScanner scan = new DepthFirstScanner();
scan.setup(start.getExecution().getCurrentHeads());
for (FlowNode f : scan) {
Expand All @@ -112,11 +112,8 @@ BlockEndNode bruteForceScanForEnd(@Nonnull BlockStartNode start) {
return null;
}




/** Do a brute-force scan for the enclosing blocks **/
BlockStartNode bruteForceScanForEnclosingBlock(@Nonnull final FlowNode node) {
synchronized BlockStartNode bruteForceScanForEnclosingBlock(@Nonnull final FlowNode node) {
FlowNode current = node;

while (!(current instanceof FlowStartNode)) { // Hunt back for enclosing blocks, a potentially expensive operation
Expand Down Expand Up @@ -157,7 +154,7 @@ BlockStartNode bruteForceScanForEnclosingBlock(@Nonnull final FlowNode node) {

@CheckForNull
@Override
public BlockEndNode getEndNode(@Nonnull final BlockStartNode startNode) {
public synchronized BlockEndNode getEndNode(@Nonnull final BlockStartNode startNode) {

String id = blockStartToEnd.get(startNode.getId());
if (id != null) {
Expand All @@ -177,7 +174,7 @@ public BlockEndNode getEndNode(@Nonnull final BlockStartNode startNode) {

@CheckForNull
@Override
public BlockStartNode findEnclosingBlockStart(@Nonnull FlowNode node) {
public synchronized BlockStartNode findEnclosingBlockStart(@Nonnull FlowNode node) {
if (node instanceof FlowStartNode || node instanceof FlowEndNode) {
return null;
}
Expand Down