improved 6507 disassembly speed

disassemby when called from normal cartridge attachment is now done the
background in a separate goroutine. this allows execution to begin
immediately without having to wait for the disassembly to finish

disasm window in the gui will show a "disassembling..." message while
the background disassembly is working

fixed symbols test data

debugger/debugger.go

@@ -1228,10 +1228,8 @@ func (dbg *Debugger) attachCartridge(cartload cartridgeloader.Loader) (e error)
 	dbg.ref.Clear()
 	dbg.counter.Clear()
 
-	err = dbg.Disasm.FromMemory()
-	if err != nil {
-		logger.Log(logger.Allow, "debugger", err)
-	}
+	// performe disassembly in the background
+	dbg.Disasm.Background(cartload)
 
 	dbg.CoProcDisasm.AttachCartridge(dbg.vcs.Mem.Cart)
 	err = dbg.CoProcDev.AttachCartridge(dbg.vcs.Mem.Cart, cartload.Filename, dbg.opts.ELF)

disassembly/decode.go

@@ -26,6 +26,9 @@ import (
 	"github.com/jetsetilly/gopher2600/logger"
 )
 
+// IMPORTANT: all functions in this file should be called in the Dissasembly
+// critical section
+
 func (dsm *Disassembly) disassemble(mc *cpu.CPU, mem *disasmMemory) error {
 	// basic decoding pass
 	err := dsm.decode(mc, mem)
@@ -216,11 +219,6 @@ func (dsm *Disassembly) blessSequence(bank int, addr uint16, commit bool) bool {
 	// mask address into indexable range
 	a := addr & memorymap.CartridgeBits
 
-	// blessing can happen at the same time as iteration which is probably
-	// being run from a different goroutine. acknowledge the critical section
-	dsm.crit.Lock()
-	defer dsm.crit.Unlock()
-
 	hasCommitted := false
 
 	// examine every entry from the starting point a. next entry determined in
@@ -301,11 +299,6 @@ func (dsm *Disassembly) blessSequence(bank int, addr uint16, commit bool) bool {
 }
 
 func (dsm *Disassembly) decode(mc *cpu.CPU, mem *disasmMemory) error {
-	// decoding can happen at the same time as iteration which is probably
-	// being run from a different goroutine. acknowledge the critical section
-	dsm.crit.Lock()
-	defer dsm.crit.Unlock()
-
 	for _, bank := range mem.banks {
 		mem.currentBank = bank.Number
 

disassembly/disassembly.go

@@ -19,6 +19,7 @@ import (
 	"fmt"
 	"strings"
 	"sync"
+	"sync/atomic"
 
 	"github.com/jetsetilly/gopher2600/cartridgeloader"
 	"github.com/jetsetilly/gopher2600/disassembly/symbols"
@@ -29,6 +30,7 @@ import (
 	"github.com/jetsetilly/gopher2600/hardware/memory/cartridge/mapper"
 	"github.com/jetsetilly/gopher2600/hardware/memory/memorymap"
 	"github.com/jetsetilly/gopher2600/hardware/television"
+	"github.com/jetsetilly/gopher2600/logger"
 )
 
 // Disassembly represents the annotated disassembly of a 6507 binary.
@@ -47,8 +49,12 @@ type Disassembly struct {
 	// emulation goroutine
 	disasmEntries DisasmEntries
 
-	// critical sectioning to protect disasmEntries
+	// critical sectioning to protect disasmEntries. the symbols table has it's
+	// own critical section
 	crit sync.Mutex
+
+	// is true if background disassembly is active
+	background atomic.Bool
 }
 
 // DisasmEntries contains the individual disassembled entries of the current ROM.
@@ -73,6 +79,7 @@ func NewDisassembly(vcs *hardware.VCS) (*Disassembly, *symbols.Symbols, error) {
 	if err != nil {
 		return nil, nil, fmt.Errorf("disassembly: %w", err)
 	}
+	dsm.background.Store(false)
 
 	return dsm, &dsm.Sym, nil
 }
@@ -121,35 +128,43 @@ func FromCartridge(cartload cartridgeloader.Loader) (*Disassembly, error) {
 	return dsm, nil
 }
 
+// Background performs a disassembly from memory but in the background
+func (dsm *Disassembly) Background(cartload cartridgeloader.Loader) {
+	go func() {
+		dsm.background.Store(true)
+		defer dsm.background.Store(false)
+		err := dsm.FromMemory()
+		if err != nil {
+			logger.Logf(dsm.vcs.Env, "disassembly background", err.Error())
+		}
+	}()
+}
+
 // FromMemory disassembles an existing instance of cartridge memory using a
 // cpu with no flow control. Unlike the FromCartridge() function this function
 // requires an existing instance of Disassembly.
 //
 // Disassembly will start/assume the cartridge is in the correct starting bank.
 func (dsm *Disassembly) FromMemory() error {
-	// unlocking manually before we call the disassmeble() function. this means
-	// we have to be careful to manually unlock before returning an error.
 	dsm.crit.Lock()
+	defer dsm.crit.Unlock()
 
 	// create new memory
 	copiedBanks, err := dsm.vcs.Mem.Cart.CopyBanks()
 	if err != nil {
-		dsm.crit.Unlock()
 		return fmt.Errorf("disassembly: %w", err)
 	}
 
 	startingBank := dsm.vcs.Mem.Cart.GetBank(cpu.Reset).Number
 
 	mem := newDisasmMemory(startingBank, copiedBanks)
 	if mem == nil {
-		dsm.crit.Unlock()
 		return fmt.Errorf("disassembly: %s", "could not create memory for disassembly")
 	}
 
 	// read symbols file
 	err = dsm.Sym.ReadDASMSymbolsFile(dsm.vcs.Mem.Cart)
 	if err != nil {
-		dsm.crit.Unlock()
 		return err
 	}
 
@@ -162,17 +177,13 @@ func (dsm *Disassembly) FromMemory() error {
 
 	// exit early if cartridge memory self reports as being ejected
 	if dsm.vcs.Mem.Cart.IsEjected() {
-		dsm.crit.Unlock()
 		return nil
 	}
 
 	// create a new NoFlowControl CPU to help disassemble memory
 	mc := cpu.NewCPU(mem)
 	mc.NoFlowControl = true
 
-	dsm.crit.Unlock()
-	// end of critical section
-
 	// disassemble cartridge binary
 	return dsm.disassemble(mc, mem)
 }
@@ -241,6 +252,11 @@ func (dsm *Disassembly) ExecutedEntry(bank mapper.BankInfo, result execution.Res
 		return e
 	}
 
+	// do not update disassembly if background disassembly is ongoing
+	if dsm.background.Load() {
+		return e
+	}
+
 	// updating an entry can happen at the same time as iteration which is
 	// probably being run from a different goroutine. acknowledge the critical
 	// section
@@ -381,9 +397,14 @@ func (dsm *Disassembly) FormatResult(bank mapper.BankInfo, result execution.Resu
 // Function will be executed with a nil argument if disassembly is not valid.
 //
 // Should not be called from the emulation goroutine.
-func (dsm *Disassembly) BorrowDisasm(f func(*DisasmEntries)) {
+func (dsm *Disassembly) BorrowDisasm(f func(*DisasmEntries)) bool {
+	if dsm.background.Load() {
+		return false
+	}
+
 	dsm.crit.Lock()
 	defer dsm.crit.Unlock()
 
 	f(&dsm.disasmEntries)
+	return true
 }

disassembly/preferences.go

@@ -50,6 +50,8 @@ func newPreferences(dsm *Disassembly) (*Preferences, error) {
 		} else {
 			p.mirrorOrigin = memorymap.OriginCart
 		}
+		dsm.crit.Lock()
+		defer dsm.crit.Unlock()
 		dsm.setCartMirror()
 		return nil
 	})
@@ -102,10 +104,9 @@ func (p *Preferences) Save() error {
 
 // setCartMirror sets the mirror bits to the user's preference. called by the
 // FxxxMirror callback.
+//
+// must be called inside the Disassembly critical section
 func (dsm *Disassembly) setCartMirror() {
-	dsm.crit.Lock()
-	defer dsm.crit.Unlock()
-
 	for b := range dsm.disasmEntries.Entries {
 		for _, e := range dsm.disasmEntries.Entries[b] {
 			if e == nil {

disassembly/symbols/canonise.go

@@ -27,8 +27,6 @@ import (
 //
 // should be called in critical section.
 func (sym *Symbols) canonise(cart *cartridge.Cartridge) {
-	defer sym.resort()
-
 	// loop through the array of canonical names.
 	//
 	// note that because Read and Write in the cpubus package are sparse

disassembly/symbols/search.go

@@ -93,7 +93,7 @@ func (sym *Symbols) SearchByAddress(addr uint16, table SearchTable) *SearchResul
 		addr, _ = memorymap.MapAddress(addr, true)
 
 		for _, l := range sym.label {
-			if s, ok := l.byAddr[addr]; ok {
+			if s, ok := l.symbols[addr]; ok {
 				return &SearchResults{
 					Table:   SearchLabel,
 					Entry:   s,
@@ -102,15 +102,15 @@ func (sym *Symbols) SearchByAddress(addr uint16, table SearchTable) *SearchResul
 			}
 		}
 	case SearchRead:
-		if s, ok := sym.read.byAddr[addr]; ok {
+		if s, ok := sym.read.symbols[addr]; ok {
 			return &SearchResults{
 				Table:   SearchRead,
 				Entry:   s,
 				Address: addr,
 			}
 		}
 	case SearchWrite:
-		if s, ok := sym.write.byAddr[addr]; ok {
+		if s, ok := sym.write.symbols[addr]; ok {
 			return &SearchResults{
 				Table:   SearchWrite,
 				Entry:   s,

disassembly/symbols/symbols.go

@@ -60,6 +60,7 @@ func (sym *Symbols) initialise(numBanks int) {
 	sym.write = newTable()
 
 	sym.canonise(nil)
+	sym.resort()
 }
 
 // should be called in critical section
@@ -71,6 +72,13 @@ func (sym *Symbols) resort() {
 	sym.write.sort()
 }
 
+// Resort all symbols tables
+func (sym *Symbols) Resort() {
+	sym.crit.Lock()
+	defer sym.crit.Unlock()
+	sym.resort()
+}
+
 // LabelWidth returns the maximum number of characters required by a label in
 // the label table.
 func (sym *Symbols) LabelWidth() int {
@@ -116,7 +124,7 @@ func (sym *Symbols) GetLabel(bank int, addr uint16) (Entry, bool) {
 
 	addr, _ = memorymap.MapAddress(addr, true)
 
-	if e, ok := sym.label[bank].byAddr[addr]; ok {
+	if e, ok := sym.label[bank].symbols[addr]; ok {
 		return e, ok
 	}
 
@@ -199,7 +207,7 @@ func (sym *Symbols) RemoveLabel(source SymbolSource, bank int, addr uint16) bool
 
 	addr, _ = memorymap.MapAddress(addr, true)
 
-	if sym.label[bank].byAddr[addr].Source != source {
+	if sym.label[bank].symbols[addr].Source != source {
 		return false
 	}
 
@@ -286,7 +294,7 @@ func (sym *Symbols) AfterLabelChange() {
 	var err error
 
 	for _, l := range sym.label {
-		for _, e := range l.byAddr {
+		for _, e := range l.symbols {
 			s = append(s, e.Symbol)
 		}
 	}
@@ -311,7 +319,7 @@ func (sym *Symbols) AfterSymbolChange() {
 	var err error
 
 	// read symbols
-	for _, e := range sym.read.byAddr {
+	for _, e := range sym.read.symbols {
 		read = append(read, e.Symbol)
 	}
 
@@ -324,7 +332,7 @@ func (sym *Symbols) AfterSymbolChange() {
 	}
 
 	// write symbols
-	for _, e := range sym.write.byAddr {
+	for _, e := range sym.write.symbols {
 		write = append(write, e.Symbol)
 	}