From 1281372a5b9cdec636588b1935b372dc08bac40f Mon Sep 17 00:00:00 2001 From: emteere <47253321+emteere@users.noreply.github.com> Date: Tue, 5 Nov 2019 08:49:10 -0500 Subject: [PATCH 1/3] GT-3262_emteere Profiler tuning fixes for visualvm --- .../src/main/java/ghidra/util/Lock.java | 34 +++++++++++-------- 1 file changed, 20 insertions(+), 14 deletions(-) diff --git a/Ghidra/Framework/Project/src/main/java/ghidra/util/Lock.java b/Ghidra/Framework/Project/src/main/java/ghidra/util/Lock.java index d271a346d3..ccacb4059b 100644 --- a/Ghidra/Framework/Project/src/main/java/ghidra/util/Lock.java +++ b/Ghidra/Framework/Project/src/main/java/ghidra/util/Lock.java @@ -1,6 +1,5 @@ /* ### * IP: GHIDRA - * REVIEWED: YES * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -17,18 +16,20 @@ package ghidra.util; /** - * Ghidra synchronization lock. This class allows creation of named locks for + * Ghidra synchronization lock. This class allows creation of named locks for * modifying tables in the Ghidra data base. This class also creates an instance - * of a global lock that must first be obtained when synchronizing using multiple - * of the named locks. + * of a global lock that must first be obtained when synchronizing using + * multiple of the named locks. */ public class Lock { private Thread owner; private int cnt = 0; + private int waiting = 0; private String name; /** * Creates an instance of a lock for synchronization within Ghidra. + * * @param name the name of this lock */ public Lock(String name) { @@ -36,8 +37,8 @@ public class Lock { } /** - * Acquire this synchronization lock. - * (i.e. begin synchronizing on this named lock.) + * Acquire this synchronization lock. (i.e. begin synchronizing on this named + * lock.) */ public synchronized void acquire() { Thread currThread = Thread.currentThread(); @@ -47,15 +48,16 @@ public class Lock { cnt = 1; owner = currThread; return; - } - else if (owner == currThread) { + } else if (owner == currThread) { cnt++; return; } try { + waiting++; wait(); - } - catch (InterruptedException e) { + } catch (InterruptedException e) { + } finally { + waiting--; } } } @@ -70,20 +72,24 @@ public class Lock { if (cnt > 0 && (owner == currThread)) { if (--cnt == 0) { owner = null; - notify(); + // This is purely to help sample profiling. If notify() is called the + // sampler can attribute time to the methods calling this erroneously. For some reason + // the visualvm sampler gets a sample more often when notify() is called. + if (waiting != 0) { + notify(); + } } - } - else { + } else { throw new IllegalStateException("Attempted to release an unowned lock: " + name); } } /** * Gets the thread that currently owns the lock. + * * @return the thread that owns the lock or null. */ public Thread getOwner() { return owner; } - } From d2726f90d7eb03e4aee2882f163019c445845765 Mon Sep 17 00:00:00 2001 From: emteere <47253321+emteere@users.noreply.github.com> Date: Wed, 6 Nov 2019 09:15:38 -0500 Subject: [PATCH 2/3] GT-3262_emteere code review changes to fix ancient flaws --- .../src/main/java/ghidra/util/Lock.java | 24 ++++++++++--------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/Ghidra/Framework/Project/src/main/java/ghidra/util/Lock.java b/Ghidra/Framework/Project/src/main/java/ghidra/util/Lock.java index ccacb4059b..e95d175f21 100644 --- a/Ghidra/Framework/Project/src/main/java/ghidra/util/Lock.java +++ b/Ghidra/Framework/Project/src/main/java/ghidra/util/Lock.java @@ -17,14 +17,14 @@ package ghidra.util; /** * Ghidra synchronization lock. This class allows creation of named locks for - * modifying tables in the Ghidra data base. This class also creates an instance - * of a global lock that must first be obtained when synchronizing using + * modifying tables in the Ghidra database. An instance of this class is used as + * a global lock that must first be obtained when synchronizing use of * multiple of the named locks. */ public class Lock { private Thread owner; - private int cnt = 0; - private int waiting = 0; + private int lockAquireCount = 0; + private int waiterCount = 0; private String name; /** @@ -45,19 +45,21 @@ public class Lock { while (true) { if (owner == null) { - cnt = 1; + lockAquireCount = 1; owner = currThread; return; } else if (owner == currThread) { - cnt++; + lockAquireCount++; return; } try { - waiting++; + waiterCount++; wait(); } catch (InterruptedException e) { + // exception from another threads notify(), ignore + // and try to get lock again } finally { - waiting--; + waiterCount--; } } } @@ -69,13 +71,13 @@ public class Lock { public synchronized void release() { Thread currThread = Thread.currentThread(); - if (cnt > 0 && (owner == currThread)) { - if (--cnt == 0) { + if (lockAquireCount > 0 && (owner == currThread)) { + if (--lockAquireCount == 0) { owner = null; // This is purely to help sample profiling. If notify() is called the // sampler can attribute time to the methods calling this erroneously. For some reason // the visualvm sampler gets a sample more often when notify() is called. - if (waiting != 0) { + if (waiterCount != 0) { notify(); } } From 9fd88806149705cbb8b336b83c816ab1cb31688e Mon Sep 17 00:00:00 2001 From: emteere <47253321+emteere@users.noreply.github.com> Date: Fri, 8 Nov 2019 12:47:55 -0500 Subject: [PATCH 3/3] GT-3262_emteere code review comment tweak --- .../Project/src/main/java/ghidra/util/Lock.java | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/Ghidra/Framework/Project/src/main/java/ghidra/util/Lock.java b/Ghidra/Framework/Project/src/main/java/ghidra/util/Lock.java index e95d175f21..7a42b51b82 100644 --- a/Ghidra/Framework/Project/src/main/java/ghidra/util/Lock.java +++ b/Ghidra/Framework/Project/src/main/java/ghidra/util/Lock.java @@ -17,9 +17,7 @@ package ghidra.util; /** * Ghidra synchronization lock. This class allows creation of named locks for - * modifying tables in the Ghidra database. An instance of this class is used as - * a global lock that must first be obtained when synchronizing use of - * multiple of the named locks. + * synchroniing modification of multiple tables in the Ghidra database. */ public class Lock { private Thread owner; @@ -48,17 +46,20 @@ public class Lock { lockAquireCount = 1; owner = currThread; return; - } else if (owner == currThread) { + } + else if (owner == currThread) { lockAquireCount++; return; } try { waiterCount++; wait(); - } catch (InterruptedException e) { + } + catch (InterruptedException e) { // exception from another threads notify(), ignore // and try to get lock again - } finally { + } + finally { waiterCount--; } } @@ -81,7 +82,8 @@ public class Lock { notify(); } } - } else { + } + else { throw new IllegalStateException("Attempted to release an unowned lock: " + name); } }