Only load floating point registers when needed

[aarzilli]

proc/*: only load floating point registers when needed

Changes implementations of proc.Registers interface and the
op.DwarfRegisters struct so that floating point registers can be loaded
only when they are needed.
Removes the floatingPoint parameter from proc.Thread.Registers.
This accomplishes three things:

1. it simplifies the proc.Thread.Registers interface
2. it makes it impossible to accidentally create a broken set of saved
registers or of op.DwarfRegisters by accidentally calling
Registers(false)
3. it improves general performance of Delve by avoiding to load
floating point registers as much as possible

Floating point registers are loaded under two circumstances:

1. When the Slice method is called with floatingPoint == true
2. When the Copy method is called

Benchmark before:

BenchmarkConditionalBreakpoints-4   	       1	4327350142 ns/op

Benchmark after:

BenchmarkConditionalBreakpoints-4   	       1	3852642917 ns/op

Updates #1549

proc: do not convert floating point registers until needed

[aarzilli]

Looks like I broke arm64, I'll have to look into it.

[chainhelen]

En, about

Benchmark before:

BenchmarkConditionalBreakpoints-4   	       1	4327350142 ns/op

Benchmark after:

BenchmarkConditionalBreakpoints-4   	       1	3852642917 ns/op

When I do benchmark (previous similar optimizations), the fluctuation is very big every time and the value is inconspicuous. Of course it must be optimized in theory.

I just want to konw how to generate reliable data. Maybe remove the maximum, remove the minimum, take the average?

[aarzilli]

Looks like I broke arm64, I'll have to look into it.

Fixed it.

When I do benchmark (previous similar optimizations), the fluctuation is very big every time and the value is inconspicuous. Of course it must be optimized in theory.

I just want to konw how to generate reliable data. Maybe remove the maximum, remove the minimum, take the average?

I don't get a lot of variance. If you are using 1.14 disable async preemption, that adds a lot of noise, otherwise change the number of iterations in issue1549.go.

[derekparker]

This is just a thin wrapper around initializing the struct. All function args become struct members with no processing done by the function. Maybe we just let the callers initialize the struct themselves?

[aarzilli]

I unexported the Regs field so that consumers of DwarfRegisters can't access it directly and have to go through the appropriate methods (that will call loadMoreCallback when needed). Philosophical reasons aside, this also made sure that I converted all accesses to Regs. However now users of DwarfRegisters can't instantiate it because they can't assign to regs.

[derekparker]

Ack, makes perfect sense.

[derekparker]

nit: I think this would read better as:

if regs.loadMoreCallback == nil {
        return nil
}
regs.loadMoreCallback()
regs.loadMoreCallback = nil
if idx >= uint64(len(regs.regs)) {
	return nil
}

[aarzilli]

Done.

[derekparker]

I think we should put this into a regs.loadMore method which can wrap the logic of checking if the callback is not nil and then setting it to nil once it's been called.

[aarzilli]

Done, see what you think.

[derekparker]

I think the name should reflect that this is selective in which registers it clears (e.g. ClearGeneralPurposeRegisters or something). Also, why be selective?

[aarzilli]

General purpose registers usually refers to rax, rbx, etc... this cleans all of them (including xmm, etc) except for PC, SP and BP. It's like this because the only place we need this is in the function call protocol.

However, you are right, this is pretty weird and ugly and I've thought of a way to do it better.