Significantly improve diffing performance and fix minor bug with bss section match percents

[LagoLunatic]

First commit optimizes diffing by moving the expensive string processing logic out of the loops being run to compare every symbol to every other symbol, and instead doing the logic just once while reading the object so that simple string equals checks can be used in the loop.

Second commit optimizes diffing more by using min in the loops instead of sorting.

Third commit optimizes this same part even further by removing the min in the loop and instead sorting all symbols once when the object is read. Moving the sorting earlier had the unintended side effect of fixing a subtle bug in diff_bss_section that I didn't know about: it assumes the bss symbols are in order and only compares their sizes, so when they're not in the right order it will report <100% if the bss symbol names don't match. Now it doesn't care about bss symbol names as long as the sizes match - this was the originally intended behavior for diff_bss_section, right?

Related: #273

[LagoLunatic]

Comparison for the bss section change:
Before:

After:

It now shows 100% even though the symbol names don't match. Seems like that was the intended behavior, though I'm not sure if that might be too lenient now?

[tomsons26]

i think it may have to do with

rather names

lbl and @ may be filtered somewhere and it didn't work before?

[LagoLunatic]

i think it may have to do with rather names

lbl and @ may be filtered somewhere and it didn't work before?

Sorry, I'm not following at all. Can you rephrase that?

To be clear I didn't change any "filtering" behavior in this PR.

[LagoLunatic]

When I mentioned the bug in diff_bss_section, I was referring to this:

objdiff/objdiff-core/src/diff/data.rs

Lines 564 to 573 in 7834185

	let left_section = &left_obj.sections[left_section_idx];
	let left_sizes = symbols_matching_section(&left_obj.symbols, left_section_idx)
	.filter_map(|(_, s)| s.address.checked_sub(left_section.address).map(|a| (a, s.size)))
	.collect::<Vec<_>>();
	let right_section = &right_obj.sections[right_section_idx];
	let right_sizes = symbols_matching_section(&right_obj.symbols, right_section_idx)
	.filter_map(|(_, s)| s.address.checked_sub(right_section.address).map(|a| (a, s.size)))
	.collect::<Vec<_>>();
	let ops = capture_diff_slices(Algorithm::Patience, &left_sizes, &right_sizes);
	let mut match_percent = get_diff_ratio(&ops, left_sizes.len(), right_sizes.len()) * 100.0;

Notice how it takes the bss symbols in the section and compares their sizes without sorting them first. This PR sorts everything when reading the object, so that's why the bug is now fixed, is what I was getting at.

[tomsons26]

I see
Have to confess #255 introduced a speed degradation.

By doing SectionKind::Data | SectionKind::Code => {
idea was to try to match the generated functions using the complex method that would compare relocs too, but locally i've had it changed up from
SectionKind::Bss => { to SectionKind::Bss | SectionKind::Code => { leaving SectionKind::Data => { as just that without Code case, it does a simple size compare then but if your static inits are in order then it shouldn't be problem
As for my objs the perf hit is rather significant

I don't know the correct solution for this so i've left it be until now

[LagoLunatic]

Are you saying the performance degradation from #255 still exists after this PR? Or is it fixed now?

This PR did move your "_$E" string processing out of the expensive loop so it might be fixed. If it's not fixed then I would need you to upload a pair of your objects so that I can see why it's slow myself, I'm not familiar with MSVC and don't have large MSVC objects to diff.

[LagoLunatic]

Oh I didn't see that you already sent objects in Discord DMs. I tried diffing them on this PR and it's more than 2x faster now, so yeah I think that issue is also resolved by the first commit of this PR.