Work at SourceForge, help us to make it a better place! We have an immediate need for a Support Technician in our San Francisco or Denver office.

Close

#476 Fix performance revealed in bug 3590540

closed-fixed
Thomas Meyer
None
5
2013-01-12
2012-12-12
Thomas Meyer
No

Don' tokenize a really long line into screen lines (Chunks) for every scroll event, when the physical line doesn't change.

Discussion

  • Alan Ezust
    Alan Ezust
    2012-12-30

    Looks good, testing it and I haven't run into problems yet. You can commit this yourself.

     
  • Alan Ezust
    Alan Ezust
    2012-12-30

    • assigned_to: nobody --> thomasmey
    • status: open --> open-accepted
     
  • Thomas Meyer
    Thomas Meyer
    2013-01-10

    Found a bug in my patch.

    This call chain:

    DisplayManager.updateScreenLineCount(int) Zeile: 659
    DisplayManager.getScreenLineCount(int) Zeile: 230
    FirstLine.reset() Zeile: 264
    DisplayManager.notifyScreenLineChanges() Zeile: 519
    JEditTextArea(TextArea).propertiesChanged() Zeile: 4845
    DisplayManager.updateScreenLineCount(int) Zeile: 659
    DisplayManager.getScreenLineCount(int) Zeile: 230
    FirstLine.reset() Zeile: 264
    DisplayManager.notifyScreenLineChanges() Zeile: 519
    JEditTextArea(TextArea).propertiesChanged() Zeile: 4845

    reveals that FirsLine.reset() calls DisplayManager.updateScreenLineCount() which detects via the screen line manager that the current screen line is invalid and calls chunkCache.getLineSubregionCount(line) and so relies on the current behaviour of always chunk the line completely new.

    The fix is easys: reset the chunkCache in this case:
    --- a/org/gjt/sp/jedit/textarea/DisplayManager.java Thu Jan 10 22:27:23 2013 +0100
    +++ b/org/gjt/sp/jedit/textarea/DisplayManager.java Thu Jan 10 22:49:42 2013 +0100
    @@ -655,6 +655,8 @@

    if(!screenLineMgr.isScreenLineCountValid(line))
    {
    + // reset chunk cache here
    + textArea.chunkCache.reset();
    int newCount = textArea.chunkCache.getLineSubregionCount(line);

    assert newCount > 0;

    So hopefully this is the only place that needs an explicit reset() call before calling chunkCache.getLineSubregionCount(line)!

    Please test the new patch, especially with the one million digits of pi file mention in the original bug report.

     
  • Thomas Meyer
    Thomas Meyer
    2013-01-12

    • status: open-accepted --> closed-fixed
     
  • Thomas Meyer
    Thomas Meyer
    2013-01-12

    Commit 22670.