fortinmike / XcodeBoost

An Xcode plugin that aims to make altering and inspecting code quick and easy.
MIT License
817 stars 86 forks source link

The highlight is really slow (especially un-highlighting) #32

Closed Mazyod closed 10 years ago

Mazyod commented 10 years ago

Hi,

Thanks for this wonderful plugin, it's truly a life saver.

The only feature that I extensively use is the highlight feature, and honestly it is really slow. It takes about a good second or two to highlight, and then same amount to remove, which really harms productivity.

Is it just me? Can I somehow profile it? (MBP RD 2012)

Thanks, Maz

fortinmike commented 10 years ago

Hi @Mazyod, I'm glad you like the plugin!

Highlighting is instantaneous on my machine (mostly the same as yours). The only thing that sometimes takes a second or two at most is un-highlighting, which is a known issue.

Are you working with very large source files? Admittedly I didn't test the highlight feature with source files that have more than a few hundred lines of code.

If you want, you can try profiling the plugin by cloning the source, setting the executable to Xcode.app in the Xcode scheme's Profiling section, then launching profiling with Time Profiler to take a look at what might be the issue.

I'm going to try profiling it on my machine soon to optimize that.

Mazyod commented 10 years ago

Thanks for the prompt reply @fortinmike!

Admittedly, the unhighlight is more glaring of an issue. Sorry, I quickly skimmed the issues, and didn't find anything related to that, my bad.

Yeah, our source files are 500 - 800 lines on average :disappointed: I am using it mainly to highlight self in order to break those block retain cycles, and self is used everywhere.

fortinmike commented 10 years ago

Thanks for reporting this, I'll take a look as soon as I find some free time. Maybe I didn't add an issue for this, no need to apologize.

I'd also gladly accept a pull request if you think this is a show-stopper and want to contribute.

Michaël Fortin www.irradiated.net

On 2014-07-03, at 08:37 AM, Mazyad Alabduljaleel notifications@github.com wrote:

Thanks for the prompt reply @fortinmike!

Admittedly, the unhighlight is more glaring of an issue. Sorry, I quickly skimmed the issues, and didn't find anything related to that, my bad.

Yeah, our source files are 500 - 800 lines on average I am using it mainly to highlight self in order to break those block retain cycles, and self is used everywhere.

— Reply to this email directly or view it on GitHub.

Mazyod commented 10 years ago

I'd really love to contribute, but I don't have much time myself to learn the code base ^^; I'll take a stab at it this weekend, though, just to try it out for a couple of hours or so.

Mazyad Alabduljaleel Sent with Sparrow (http://www.sparrowmailapp.com/?sig)

On Thursday, July 3, 2014 at 5:36 PM, fortinmike wrote:

Thanks for reporting this, I'll take a look as soon as I find some free time. Maybe I didn't add an issue for this, no need to apologize.

I'd also gladly accept a pull request if you think this is a show-stopper and want to contribute.

Michaël Fortin
www.irradiated.net (http://www.irradiated.net)

On 2014-07-03, at 08:37 AM, Mazyad Alabduljaleel <notifications@github.com (mailto:notifications@github.com)> wrote:

Thanks for the prompt reply @fortinmike!

Admittedly, the unhighlight is more glaring of an issue. Sorry, I quickly skimmed the issues, and didn't find anything related to that, my bad.

Yeah, our source files are 500 - 800 lines on average I am using it mainly to highlight self in order to break those block retain cycles, and self is used everywhere.


Reply to this email directly or view it on GitHub.

— Reply to this email directly or view it on GitHub (https://github.com/fortinmike/XcodeBoost/issues/32#issuecomment-47930039).

dtrembovetski commented 10 years ago

Same here with extremely slow highlight times, several seconds with spinning beach ball, essentially makes this feature unusable, unfortunately (especially after being used to Eclipse/IntelliJ automatic instantaneous highlight of a word under the cursor).

dtrembovetski commented 10 years ago

One possible suggestion on limiting the effect is to only highlight the symbol in the current scope (don't know if there's an easy way to detect the scope though).

fortinmike commented 10 years ago

@dtrembovetski Because XcodeBoost has no true internal model of the code itself, there is no way to reliably check for a symbol's scope. Issue #1 (using Clang to make sense of source code) would probably be the best solution. Any hints on how to get started using Clang (admittedly a pretty complex subject) would be appreciated. The feature/clang-playground branch dabbles in this a bit.

But this is not the cause of the slowdown, AFAIK. Rather, it seems to be linked to the way the highlighting attributes are applied and removed from the NSTextView subclass (i.e. the code editor). If some of you want to take a look, -[MFSourceTextViewManipulator highlightSelectedSymbols / removeAllHighlighting] might be a good starting point for profiling using Instruments. I'm not a Cocoa Text Architecture pro, so I'm probably doing something in a sub-optimal way.

-[NSTextStorage enumerateAttribute:inRange:options:usingBlock:] was especially slow in my brief testing.

Unfortunately I have no time at all to fix this at the moment, but be assured I will come back to this eventually. With that said, a pull request or some experimentation results would be awesome!

Mazyod commented 10 years ago

I changed the enumeration code in the removeAllHighlighting method to this:

[textStorage removeAttribute:XBHighlightColorAttributeName range:documentRange];

Significantly improved the unhighlight performance.

Since you can specify your own attribute names, it might be worth assigning more dynamic attribute names, instead of storing and enumerating the ranges.

i.e.:

[textStorage removeAttribute:[NSString stringWithFormat:@"%@%ld", XBHighlightColorAttributeName, (long)index] range:documentRange];

How does that sound?

Mazyod commented 10 years ago

OK, scratch that. I found your fix, pretty straight forward.

It was clear from the profiling that the NSMutableAttributedString was doing heavy redrawing for each attribute change, even though they should all be done in a single shot. I tried [NSAnimationContext beginGrouping] to try and add animation as well as hopefully defer the drawing, didn't help. But then, I stumbled across -[NSMutableAttributedString beginEditing], which was a bingo for this use case.

It's already working on the remove all method, I just wanted to apply it on the highlight as well before submitting a PR :)

fortinmike commented 10 years ago

Great, thanks @Mazyod ! Actually, I read a bit about these exact methods (begin/endEditing) after reading @dtrembovetski's comments. Seems like I was on the right track!

Thanks a lot for your time. Many will benefit! I'm waiting for your PR.

Mazyod commented 10 years ago

Haha, yup. No problem, I am happy to be able to contribute, even a little, to this great project :)

fortinmike commented 10 years ago

Thanks again, I'll take a look at your PR after my workday. It'll be available for everyone soon enough! :)