add search function for source code was: cache source code in vector for easier access in search function

[lievenhey]

I updated the SourceCodeModel to only include the function that is disassembled. Until now the whole file was included which wasted a lot of performance since KSyntaxHighlighter was run on the whole file.

[milianw]

I thought that was by design, to ensure the syntax highlighting is correct? how does this behave when we have code like

int
/* some comment */
MY_MACRO
actualFunction(
   int a,
   int b
)
throw(...)
{
}

[lievenhey]

The debuginfos mark the line with { as the start of the function. So unless you manage to break KSyntaxHighlighting in one line we are save.

I thought about adding some logic to detect the start of the declaration, but there are too many edge cases to correctly detect this. The logic also would only work for C / C++ and would likely break if some other languages are used.
That is the reason why line 0 contains the symbol name.

[milianw]

The debuginfos mark the line with { as the start of the function. So unless you manage to break KSyntaxHighlighting in one line we are save.

This style is pretty common, and would then break, or?

longTypeName foo(int bar,
                 double foo) {

}

I thought about adding some logic to detect the start of the declaration, but there are too many edge cases to correctly detect this. The logic also would only work for C / C++ and would likely break if some other languages are used. That is the reason why line 0 contains the symbol name.

Exactly, which is why I actually like the old approach of just highlighting the full file. it's not that slow, or?

[GitMensch]

I commonly have to wait around 5 seconds (small programs) to 30 seconds (big ones) - not tested so far with huge ones.
But in my case those are actually nearly all in one (generated) big function....

I personally don't see this as a big issue (other then #468 and much more #457 which do solve big issues I nearly always stumble over when working with hotspot).

I'd be most concerned what this change means to inline functions and functions that heavily use macros.
@lievenhey Would you mind sharing "before" and "after" results of your added test as image?

[GitMensch]

Testing with current appImage yields in console flooded with "HSV parameters out of range" when hovering over the source code.
Edit: this also happens with appimage from CI "46".

[lievenhey]

I commonly have to wait around 5 seconds (small programs) to 30 seconds (big ones) - not tested so far with huge ones. But in my case those are actually nearly all in one (generated) big function....

that's weird. It should need more than a few milliseconds (my machine has no problem highlighting a file with 4.4 million lines in less than a second). Can you send us an example so we can profile this?

Testing with current appImage yields in console flooded with "HSV parameters out of range" when hovering over the source code.

That's even weirder since the parameters a clamped to the allows range.

[GitMensch]

y machine has no problem highlighting a file with 4.4 million lines in less than a second

Note that I'm using the appimage only which is currently limited to xcb - do you have this nice highlighting performance (and included disassembly) in less than a second using an appimage, too?

[note: the big files I've tested were all over network - client with xserver, server "in the cloud", I guess that's the main reason for the performance I've seen].

... but if huge files take less then a second, how is the original point of this PR

Until now the whole file was included which wasted a lot of performance since KSyntaxHighlighter was run on the whole file.

an issue?

[lievenhey]

This style is pretty common, and would then break, or?

longTypeName foo(int bar,
                 double foo) {

}

that doesn't brake syntax highlighting. It also resists

longTypeName foo(int bar, double foo) /*
*/ {

}

[lievenhey]

Until now the whole file was included which wasted a lot of performance since KSyntaxHighlighter was run on the whole file.

an issue?

I didn't really profile this but it seemed that highlighting a file with 1000+ lines to show a function with 10 lines. But now I feel stupid for spending time on something that has a negligent impact on performance.

Does this patch help in your case?

[GitMensch]

Does this patch help in your case?

Rechecked: With both CI builds 49 (current from this PR) and 50 (current from master) it takes ~27 seconds to open the disassembly on a "bad" sample with visible text. With master I've then seen ~1 second then the black text gets colored, with this PR it was nearly instant after the black-only text - but the ~27 seconds before coloring were identical.

As noted: I think the issue is xcb from "cloud" to local xserver.

[GitMensch]

Exactly, which is why I actually like the old approach of just highlighting the full file. it's not that slow, or?

I think the "slowness" is not that bad for the highlighting of the complete file.
But I do think the currently "everything highlighted, but often only minor parts - related to the current function - are assigned with costs" is sometimes confusing. Not sure if we can show them for the complete file / part that is highlighted.

[milianw]

two minor nits, otherwise lgtm

[milianw]

uhm did you push the wrong branch to this review? the discussion we had here doesn't match the code I just reviewed, or?

[lievenhey]

I removed some stuff since it didn't work as expected.

[GitMensch]

just tested the current AppImage - Search works fine in general but I did not found a way to enable search other than CTRL+F, ideally there is a GUI option to start that - and ideally the same function also enables to "close" the search bar.

One "gotcha": the current version never outputs a "no (more) occurence(s) found" mesage - instead clicking on "search" just goes to the next line.

I guess that would be much less useful for most people - but waht about an optional search bar for the assembly window?

[lievenhey]

Thanks, I completely forgot the buttons.

I guess that would be much less useful for most people - but waht about an optional search bar for the assembly window?

That would be interesting with the keyboard shortcuts. Will see what I can do about them. Probably something like mouse is above disassembly -> disassembly search otherwise source code search.

[GitMensch]

I guess that would be much less useful for most people - but waht about an optional search bar for the assembly window?

That would be interesting with the keyboard shortcuts. Will see what I can do about them. Probably something like mouse is above disassembly -> disassembly search otherwise source code search.

Either that or simply have: CTRL+F-> code search; CTRL+SHIFT+F -> disassembly search.

[milianw]

no, we should search in the focused widget via the correct shortcut context, and let the user use Tab to switch focus as needed

[GitMensch]

we should search in the focused widget via the correct shortcut context, and let the user use Tab to switch focus as needed

agreed in principle - but can we then please:

• focus to source code widget when entering the tab
• provide a fast way to switch between source code, disassembly and timeline widget

Background: the tab key switches between every control which includes, additional to those three:

• fast navigate
• syntax highlighting (2 times)
• new: search (2 times)
• timeline search
• timeline event source

BTW: CTRL+F doesn't position to search in timeline currently

[milianw]

I believe this code here could be greatly clarified by hoisting it into a function that takes the range as iterators and returns a result index (or -1 if nothing is found).

then you call that with either forward or backward iterators. my hunch is that the code would be much more readable then since you'd only have to take care of the direction when calling the function, but not inside it.

anyhow, leave it as-is for now, I'll try to clean it up myself then

[milianw]

we should search in the focused widget via the correct shortcut context, and let the user use Tab to switch focus as needed

agreed in principle - but can we then please:

* focus to source code widget when entering the tab

* provide a fast way to switch between source code, disassembly and timeline widget

Background: the tab key switches between every control which includes, additional to those three:

* fast navigate

* syntax highlighting (2 times)

* new: search (2 times)

* timeline search

* timeline event source

BTW: CTRL+F doesn't position to search in timeline currently

I think what you are looking for here, besides proper tab order (which is probably not a given, have to double check): ALT-modifiers to trigger quick focus changes. That way, you could press ALT+... to focus e.g. the assembly then CTRL+F to search in there. and then ALT+... to focus the source code followed by another CTRL+F to search in that one then.

[milianw]

would be cool if you can add the missing typeinfo and then submit it