D17437: SearchBar: Avoid malfunction with very large files

[Christoph Cullmann]

cullmann abandoned this revision.

REPOSITORY
  R39 KTextEditor

REVISION DETAIL
  https://phabricator.kde.org/D17437

To: cullmann, #ktexteditor, #vdg, loh.tar
Cc: cullmann, kwrite-devel, kde-frameworks-devel, #ktexteditor, hase, michaelh, 
ngraham, bruns, demsking, sars, dhaumann

D17437: SearchBar: Avoid malfunction with very large files

[Christoph Cullmann]

cullmann commandeered this revision.
cullmann edited reviewers, added: loh.tar; removed: cullmann.
cullmann added a comment.


  We have cancel now ;=)

REPOSITORY
  R39 KTextEditor

REVISION DETAIL
  https://phabricator.kde.org/D17437

To: cullmann, #ktexteditor, #vdg, loh.tar
Cc: cullmann, kwrite-devel, kde-frameworks-devel, #ktexteditor, hase, michaelh, 
ngraham, bruns, demsking, sars, dhaumann

D17437: SearchBar: Avoid malfunction with very large files

[Christoph Cullmann]

cullmann added a comment.


  Ok, I would prefer then if you retract that and submit the cancel thingy.
  
  Btw., thanks for all the nice improvements that are submitted!
  Such fixes are highly appreciated.
  
  If you think a review is not done timely, no issue to e.g. poke me.

REPOSITORY
  R39 KTextEditor

REVISION DETAIL
  https://phabricator.kde.org/D17437

To: loh.tar, #ktexteditor, #vdg, cullmann
Cc: cullmann, kwrite-devel, kde-frameworks-devel, #ktexteditor, hase, michaelh, 
ngraham, bruns, demsking, sars, dhaumann

D17437: SearchBar: Avoid malfunction with very large files

[loh tar]

loh.tar added a comment.


  In this state was this patch only a simple attempt to avoid the worst case. 
  With your new improvements is it much less likely to happen, but still 
possible. I do not like to insist of this patch it's only an offer, which could 
be improved.
  
  BTW I'm working on your suggestion to add a (functioning) Cancel button. With 
that, this patch may become more obsolete ;-)

REPOSITORY
  R39 KTextEditor

REVISION DETAIL
  https://phabricator.kde.org/D17437

To: loh.tar, #ktexteditor, #vdg, cullmann
Cc: cullmann, kwrite-devel, kde-frameworks-devel, #ktexteditor, hase, michaelh, 
ngraham, bruns, demsking, sars, dhaumann

D17437: SearchBar: Avoid malfunction with very large files

[Christoph Cullmann]

cullmann added a comment.


  I think the current state after the improvements in the other bug is good 
enough to avoid such a message.
  Would it be ok for you to abandon this?

REPOSITORY
  R39 KTextEditor

REVISION DETAIL
  https://phabricator.kde.org/D17437

To: loh.tar, #ktexteditor, #vdg, cullmann
Cc: cullmann, kwrite-devel, kde-frameworks-devel, #ktexteditor, hase, michaelh, 
ngraham, bruns, demsking, sars, dhaumann

D17437: SearchBar: Avoid malfunction with very large files

[Christoph Cullmann]

cullmann added a comment.


  I am still not that happy with a hard limit, I will first work a bit on the 
root causes: that the speed sucks ;=)
  
  https://phabricator.kde.org/D17441

REPOSITORY
  R39 KTextEditor

REVISION DETAIL
  https://phabricator.kde.org/D17437

To: loh.tar, #ktexteditor, #vdg, cullmann
Cc: cullmann, kwrite-devel, kde-frameworks-devel, #ktexteditor, hase, michaelh, 
ngraham, bruns, demsking, sars, dhaumann

D17437: SearchBar: Avoid malfunction with very large files

[loh tar]

loh.tar added a comment.


  Would be good you take the time and do that test @cullmann When you then 
think it is still useful to soften that limit let me know how.
  To become that code more smart as you suggest I have right now not the mood, 
sorry.
  I think this patch is good enough to avaoid the biggest trouble.

REPOSITORY
  R39 KTextEditor

REVISION DETAIL
  https://phabricator.kde.org/D17437

To: loh.tar, #ktexteditor, #vdg, cullmann
Cc: cullmann, kwrite-devel, kde-frameworks-devel, #ktexteditor, hase, michaelh, 
ngraham, bruns, demsking, sars, dhaumann

D17437: SearchBar: Avoid malfunction with very large files

[Christoph Cullmann]

cullmann added a comment.


  For the: I like to suggest to get rid of most of that S code and use 
instead code from the Search plugin. There is the S done in an own thread and 
did not block the UI.
  
  I would for sure like to have batched search/replace, e.g. that one does the 
stuff in chunks via single-shot timers and doesn't block the UI for e.g. 
incremental search that would be really nifty.
  
  That would allow people to abort long running stuff, too.
  
  (even no threads needed, just batch the stuff in e.g. 1000 line chunks)
  
  I would really love to have this improved, I think there are some bugs around 
that wish for non-blocking search/replace, too, like other proper editors have 
(and we not, shame on us)

REPOSITORY
  R39 KTextEditor

REVISION DETAIL
  https://phabricator.kde.org/D17437

To: loh.tar, #ktexteditor, #vdg, cullmann
Cc: cullmann, kwrite-devel, kde-frameworks-devel, #ktexteditor, hase, michaelh, 
ngraham, bruns, demsking, sars, dhaumann

D17437: SearchBar: Avoid malfunction with very large files

[loh tar]

loh.tar added a comment.


  - The mentioned crash could I not reproduce, but after 5 million hits and a 
felt eternity I had killed Kate
  - There is an issue now with the wrap message, where is now the same close 
button. Any ideas what's wrong?
  - I like to suggest to get rid of most of that S code and use instead code 
from the Search plugin. There is the S done in an own thread and did not 
block the UI.

REPOSITORY
  R39 KTextEditor

REVISION DETAIL
  https://phabricator.kde.org/D17437

To: loh.tar, #ktexteditor, #vdg, cullmann
Cc: cullmann, kwrite-devel, kde-frameworks-devel, #ktexteditor, hase, michaelh, 
ngraham, bruns, demsking, sars, dhaumann

D17437: SearchBar: Avoid malfunction with very large files

[Christoph Cullmann]

cullmann requested changes to this revision.
cullmann added a comment.
This revision now requires changes to proceed.


  Hmm, I don't like such hard limits.
  e.g. in our company, more or less all machines have 32 or 64 GB of memory and 
that should work fine.
  (not that I say we should use that much memory).
  
  I would more like either a fix for the memory hogging or a soft-limit, like 
you report "oh, we already do x replacements, that might take a lot of 
memory, do you want to continue? x remember that choice..)
  
  That will rescue people that replace stuff by accident and still let people 
with "a lot" memory use the tool like they want ;=)

REPOSITORY
  R39 KTextEditor

REVISION DETAIL
  https://phabricator.kde.org/D17437

To: loh.tar, #ktexteditor, #vdg, cullmann
Cc: cullmann, kwrite-devel, kde-frameworks-devel, #ktexteditor, hase, michaelh, 
ngraham, bruns, demsking, sars, dhaumann

D17437: SearchBar: Avoid malfunction with very large files

[loh tar]

loh.tar created this revision.
loh.tar added reviewers: KTextEditor, VDG.
Herald added projects: Kate, Frameworks.
Herald added subscribers: kde-frameworks-devel, kwrite-devel.
loh.tar requested review of this revision.

REVISION SUMMARY
  This is only a workaround to avoid cases where Kate can crash due to a
  run out of memory.
  

  
  CCBUG: 333517

TEST PLAN
  Search warning
  F6464367: 1544290032.png 
  Replace warning
  F6464368: 1544290055.png 

REPOSITORY
  R39 KTextEditor

REVISION DETAIL
  https://phabricator.kde.org/D17437

AFFECTED FILES
  src/search/katesearchbar.cpp

To: loh.tar, #ktexteditor, #vdg
Cc: kwrite-devel, kde-frameworks-devel, #ktexteditor, hase, michaelh, ngraham, 
bruns, demsking, cullmann, sars, dhaumann