python / cpython

The Python programming language
https://www.python.org
Other
63.37k stars 30.33k forks source link

IDLE text squeezer is too aggressive and is slow #79377

Closed rhettinger closed 5 years ago

rhettinger commented 5 years ago
BPO 35196
Nosy @rhettinger, @terryjreedy, @taleinat, @mr-nfamous, @grantjenks, @miss-islington, @tirkarthi
PRs
  • python/cpython#10454
  • python/cpython#11541
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields: ```python assignee = 'https://github.com/terryjreedy' closed_at = created_at = labels = ['3.8', 'expert-IDLE', 'type-bug', '3.7'] title = 'IDLE text squeezer is too aggressive and is slow' updated_at = user = 'https://github.com/rhettinger' ``` bugs.python.org fields: ```python activity = actor = 'terry.reedy' assignee = 'terry.reedy' closed = True closed_date = closer = 'terry.reedy' components = ['IDLE'] creation = creator = 'rhettinger' dependencies = [] files = [] hgrepos = [] issue_num = 35196 keywords = ['patch'] message_count = 24.0 messages = ['329507', '329513', '329538', '329543', '329546', '329555', '329556', '329557', '329558', '329559', '329560', '329579', '329580', '329588', '329645', '329651', '329652', '329718', '332881', '332883', '333556', '333557', '333865', '334547'] nosy_count = 7.0 nosy_names = ['rhettinger', 'terry.reedy', 'taleinat', 'bup', 'grantjenks', 'miss-islington', 'xtreak'] pr_nums = ['10454', '11541'] priority = 'normal' resolution = 'fixed' stage = 'resolved' status = 'closed' superseder = None type = 'behavior' url = 'https://bugs.python.org/issue35196' versions = ['Python 3.7', 'Python 3.8'] ```

    rhettinger commented 5 years ago

    The squeezed text is impairing IDLE's usability for teaching purposes. Typing help() on any built-in type such as str immediately results in a squeeze-button rather than displaying help. The same is true for showing lines from a file read or from a URL.

    I recommend showing the first 50 to 100 lines and then squeezing the remainder.

    Also, I think this may be the logic that is slowing down successive print calls in a loop. Try running:

        for i in range(500):
            print(i, sep=' ')

    or even:

        for i in range(500):
            print(i, i**2)

    The output has noticeably slow delays between successive print() calls.

    taleinat commented 5 years ago

    The squeezed text is impairing IDLE's usability for teaching purposes.

    I sincerely hoped it would achieve the opposite! I'm happy to do any work necessary to improve its usability in this context.

    The auto-squeezing can be "disabled" easily by setting the minimum # lines to a high number in the config dialog.

    Typing help() on any built-in type such as str immediately results in a squeeze-button rather than displaying help. The same is true for showing lines from a file read or from a URL.

    A quick double-click will expand the "Squeezed text" label. Also, right-click -> "View" will open the output in a viewer window; for long "help(str)" output I find this better than having it in the midst of the normal output.

    Also, I think this may be the logic that is slowing down successive print calls in a loop.

    Can you give more details? In comparison to what do you find it slow? I did a quick comparison between a recent 3.8.0a build and 3.7.0 (without Squeezer) on a Win10 machine, and both seemed to take the same time to run those loops (~3.5 seconds for the first loop, \~4.5 seconds for the second loop).

    rhettinger commented 5 years ago

    The auto-squeezing can be "disabled" easily by setting the minimum # lines to a high number in the config dialog.

    When I teach Python, it is unreasonable to have to have every learner reconfigure IDLE away from usable defaults. The squeezing is somewhat jarring and is almost never what we want in live demos. It is rarely that I don't immediately have to unsqueeze the output.

    Can you give more details?

    Try this and watch it crawl:

    >>> print(*range(500))

    I don't know whether squeezing is the culprit, but something is causing 1970s print performance.

    serhiy-storchaka commented 5 years ago
    terryjreedy commented 5 years ago

    [I wrote the following after Tal's first response, before reading Raymond's second post in response to Tal.]

    The October releases were deficient in only documenting Squeezer in the IDLE section of "What's New in Python X.Y" and a News entry (copied to Help => About IDLE => News). I don't intend to repeat mistake like this.

    As part of working through the backlog of IDLE doc issues, I have since added the following. """ Startup and code execution [existing section] ... User output in Shell [new subsection] ... Shell has a special facility for squeezing output lines down to a ‘Squeezed text’ label. This is done automatically for output over N lines (N = 50 by default). N can be changed in the PyShell section of the General page of the Settings dialog. Output with fewer lines can be squeezed by right clicking on the output. This can be useful [for] lines long enough to slow down scrolling.

    Squeezed output is expanded in place by double-clicking the label. It can also be sent to the clipboard or a separate view window by right-clicking the label.
    """

    I just noticed the missing 'for'. I am thinking of rewriting the sentence as "This can be useful when lines are so long that they make scrolling slower."

    Adding to what Tal said: IDLE's calltips replace some uses of help() in standard interactive Python. For the rest, I think being able to move hundreds of lines out of the REPL and into a separate persistent window, which that can be moved at least partly aside from Shell, improves IDLE's usability for teaching. Such blobs of text make it hard to scroll back to see previous entries and responses.

    Overall, partial squeezing does not seem like a great idea to me. It only partially fixes the scrolling issue. It would complicate both the implementation and use of squeezing, especially for viewing the whole text outside of Shell. I looked at the first 100 lines of help(str) and 65 (after the first 15) list the generic dunder methods. I think showing all either in Shell or a text view is better.

    taleinat commented 5 years ago

    If you write to stdout by small chunks, it adds a large overhead to every write() call.

    While I agree that there is great room for optimization in Squeezer's interception of write(), it doesn't appear to have a noticeable effect in such cases, e.g. in the examples provided by Raymond in the first comment. In my testing, even removing the write() interception entirely doesn't have an effect I can notice.

    I'm happy to optimize it anyways, there is plenty of "low hanging fruit" there.

    terryjreedy commented 5 years ago

    On my machine, 2.7.15 (without squeezing) and 3.7.1 (with squeezing) IDLE results (average seconds).

    from timeit import timeit
    timeit("print('nnn '*500)", number=10)  # Exp1: .0357, .0355
    timeit("for i in range(500): print(i)", number=4)  # Exp2: 1.45, 1.70
    timeit("print(*range(500))", number=4)  # Exp3: about 5*, 4.85

    Serhiy's first comment is about 500 very short lines (experiment 2) not being squeezed. This surprised me. Tal?

    Experiments 2 versus 1 illustrate Serhiy's 2nd comment. Experiements 3 (*range) versus 2 show that repeated writes to the same line are even slower.

    There is a known issue with tk Text and long lines, and 2000 chars is more than long. In fact, using range(100, 100+n) (to have a uniform 4 chars per number), the slowdown shows by n=200 (800 chars). My previous experiments have also shown that 'long' starts somewhere less than 1000. tk 8.7, in alpha or beta stage, reportedly has a re-written Text widget that improves this issue.

    taleinat commented 5 years ago

    Serhiy's first comment is about 500 very short lines (experiment 2) not being squeezed. This surprised me. Tal?

    Indeed, this is not currently supported. This is possible, it would just complicate the write() interceptor and require the new ability to update an existing "Squeezed text" label. Terry, just say the word and I'll get working on it.

    taleinat commented 5 years ago

    On my machine, 2.7.15 (without squeezing) and 3.7.1 (with squeezing) IDLE results (average seconds).

    from timeit import timeit timeit("print('nnn '*500)", number=10) # Exp1: .0357, .0355 timeit("for i in range(500): print(i)", number=4) # Exp2: 1.45, 1.70 timeit("print(range(500))", number=4) # Exp3: about 5, 4.85

    Comparing 3.7.0 to current master, I'm seeing about a 4% slowdown on the second experiment. That's significant, but probably not what Raymond or Serhiy are worried about. Regardless, I've nearly got a PR with an optimization ready.

    rhettinger commented 5 years ago

    Adding to what Tal said: IDLE's calltips replace some uses of help() in standard interactive Python. For the rest, I think being able to move hundreds of lines out of the REPL and into a separate persistent window, which that can be moved at least partly aside from Shell, improves IDLE's usability for teaching. Such blobs of text make it hard to scroll back to see previous entries and responses.

    I respectfully disagree. I've used IDLE for teaching over 200 days per year for the past seven years. It is becoming less and less usable and more and more buggy as it ventures beyond its original goals of being a simple, basic interactive development environment.

    We're still getting double and triple spacing after a syntax error and that survives a shell-restart. So, I have to close and start new session logs several times per day.

    The slow printing makes students think that Python is slow, so I switch to the command-line to show that the output can be almost instant.

    The tooltips sometimes show the print() function tooltip no matter what is being editted and it persists on the screen as your typing the whole expression. This is a significant visual distraction from the actual code.

    The squeezer causes frequent breaks in a train of thought because when I ask Python to display something, I then have to switch windows and click the unsqueeze.

    For years, I get random paste-clipboard effects in the middle of typing lines in the interactive shell session. My only defense is to type Cntl-Z to undo the splatter so that I can continue with my demos uninterrupted.

    I show people how to edit code in one window, press F5 to see and debug the results at the interactive prompt. Seeing both at the same time on side-by-side screens makes for an effective workflow. However, recently a student got into a new-tabbed mode and there didn't seem to be any way to turn it off. It became impossible to see code and output or error messages at the same time. This devastated the learner's experience.

    The redesign of the configuration fonts/tabs window was not an improvement. We need to get rid of the slider for indentation width (it attracts newcomers like a moth to flame). Instead, we need a slider for the font-size which is usually the very first thing people need to change. The giant window for font samples is cute and mostly useless.

    I don't know if it is possible, but it would be great to filter the font sets to only show monospaced fonts. Students setting to Arial degrade their Python experience without ever realizing why.

    People need to be able to edit quickly. On the Mac, Cntl-E does the right thing and goes to the end of a line, but Cntl-A goes to the beginning, even before the PS1 prompt. This a low quality experience (readline is smart-enough not to do that at the command-line). I can make a custom key set and remap Cntl-A to beginning-of-line but it is a PITA to have to get a whole classroom of people to do this every week. It should be the default.

    When started from a terminal session, IDLE emits some warnings such as " Warning: config.IdleConf.GetThemeDict - problem retrieving theme element 'context-background' from theme 'ttmmmmmmpp'" Note the odd theme name and doubled letters. Likewise, on the configure window for keys the custom key set name shows up as 'bbllttiinn'. I don't use this but the letters are oddly doubled there a well.

    Tab completion sometimes works and sometimes doesn't. When typing in text blocks (triple quoted strings) the tab key sometimes indents and sometimes starts inserting "IndexError" or some other random text".

    FWIW, I think the squeezer focused on the wrong problem. In general, I've never had a problem with too many *lines of output. You can always Ctnl-C if printing a long file or somesuch. The real problem with IDLE was excessive wide output on a *single line (i.e. look at the repr for a recently read dataset or long repr for a list). That tends to cripple IDLE because line wrap logic seems to be computationally expensive for scrolling text windows. The effect persists even after the offending text has scrolled off and is no longer visible.

    For feature requests, there's only a handful of things that I need to improve the experience:

    Some other ideas:

    Two other recurring usability problems

    Hope you all find this suggestion list to be useful.

    rhettinger commented 5 years ago
    rhettinger commented 5 years ago
    rhettinger commented 5 years ago
    tirkarthi commented 5 years ago

    Just adding the open issues given Raymond's message for reference. A lot of the IDLE issues have patches and since we moved to GitHub they were not updated. Thanks to Tal for converting some patches togit PRs. I would like to help with minor issues (changing Untitled to untitled) to get familiarity with the IDLE codebase and testing given limited bandwidth from the team of Terry, Tal and Cheyrl.

    There has also been a long standing open request to have a hot key to run linting or code formatting tools directly from IDLE. For beginners, it is too painful to have to try to coordinate between these command line tools and the IDLE editing environment. Some basic integration of the two seems like it would be a straight-forward task.

    https://bugs.python.org/issue21880

    Cntl-plus and Cntl-minus to change the font size.

    https://bugs.python.org/issue17642 (Seems to work with Turtledemo)

    An easy way to turn-on and off line numbering with off as the default. I don't personally need this, but some learner will request it once each week.

    https://bugs.python.org/issue17535

    A hotkey to clear the entire text window.

    https://bugs.python.org/issue6143

    taleinat commented 5 years ago

    Raymond, thanks for bringing up all of these issues. This kind of input from people using IDLE extensively for teaching is extremely useful. I'll leave it to Terry to decide how to manage this list, but I promise to do my best (with my limited time) to resolve the worst of these.

    A few quick notes:

    However, recently a student got into a new-tabbed mode and there didn't seem to be any way to turn it off. It became impossible to see code and output or error messages at the same time. This devastated the learner's experience.

    This sounds like macOS's new "Prefer tabs when opening documents" system preference, which breaks IDLE in various ways; see bpo-34864. It should be disabled when using IDLE.

    FWIW, I think the squeezer focused on the wrong problem. In general, I've never had a problem with too many *lines of output. [...] The real problem with IDLE was excessive wide output on a *single line [...] That tends to cripple IDLE because line wrap logic seems to be computationally expensive for scrolling text windows. The effect persists even after the offending text has scrolled off and is no longer visible.

    This is exactly the main rationale for Squeezer; it takes wrapping into account when counting lines precisely for this reason. This may not be working properly in some cases due to bpo-35208, which I discovered only yesterday and for which a PR is now ready with a fix.

    rhettinger commented 5 years ago

    This sounds like macOS's new "Prefer tabs when opening documents" system preference, which breaks IDLE in various ways; see bpo-34864. It should be disabled when using IDLE.

    Thanks for the link. I couldn't figure out what was happening with the student's computer and why it only happened to one person.

    rhettinger commented 5 years ago

    By the way, I really appreciate the work you all are putting into IDLE. It can definitely benefit from some love and attention.

    taleinat commented 5 years ago

    By the way, I really appreciate the work you all are putting into IDLE. It can definitely benefit from some love and attention.

    Thanks for the kind words, Raymond!

    df79943f-4aee-4531-a00d-c6b12816eb70 commented 5 years ago

    Not 100% sure if it's appropriate to post this here... so sorry if not.

    So anyway, the _MAX_COLS and _MAX_LINE constants used for get_argspec seem like they were intended to limit the generated text tips to at most 5 rows, 85 characters wide, which makes sense, but isn't what happens at all.

    Easy to just post an example of how the call signature isn't limited in any meaningful way, which can easily lead to a call tip millions of character long that obviously cannot be rendered and can maybe cause crashes:

    # freshly started repl session
    >>> if 1:
            from idlelib.calltips import get_argspec
            G = globals()
            @get_argspec
            def func(x, d=G): pass
            print('len of func signature:', len(func))
            print(f'len(repr(globals())): {len(repr(G)):_} ({len(G)} globals)')
    
    len of func signature: 564
    len(repr(globals())): 899 (10 globals)
    >>> from numpy import *
    >>> if 1:
            from idlelib.calltips import get_argspec
            G = globals()
            @get_argspec
            def func(x, d=G): pass
            print('len of func signature:', len(func))
            print(f'len(repr(globals())): {len(repr(G)):_} ({len(G)} globals)')
    ...
    len of func signature: 45524
    len(repr(globals())): 92_488 (604 globals)
    taleinat commented 5 years ago

    Hi Dan,

    Your report is unrelated to this Squeezer-related issue, but thanks for reporting it! I've created a new issue for what you've reported, see bpo-35641.

    taleinat commented 5 years ago

    New changeset 39a33e99270848d34628cdbb1fdb727f9ede502a by Tal Einat in branch 'master': bpo-35196: Optimize Squeezer's write() interception (GH-10454) https://github.com/python/cpython/commit/39a33e99270848d34628cdbb1fdb727f9ede502a

    miss-islington commented 5 years ago

    New changeset 47bd7770229b5238a438703ee1d52da2e983ec9e by Miss Islington (bot) in branch '3.7': bpo-35196: Optimize Squeezer's write() interception (GH-10454) https://github.com/python/cpython/commit/47bd7770229b5238a438703ee1d52da2e983ec9e

    taleinat commented 5 years ago

    The recently merged PR python/cpython#54663 significantly reduced the overhead of Squeezer's write() interception. The overhead should now be entirely insignificant.

    IMO that deals with the "... and is slow" part of this issue. We've still to decide whether the auto-squeezing is "too aggressive".

    I'll mention again that Raymond has brought up several additional important issues in the comments, that IMO should be processed into new issues and/or a roadmap for IDLE. It's Terry's decision how to proceed, but I'll be happy to help with whatever direction he chooses.

    terryjreedy commented 5 years ago

    I think any further work on IDLE print speed should look at the entire path from print('x') to 'x' appearing in IDLE's Shell. Where does the time go, what might be sped up?

    I no longer think auto-squeezing should consider more than a single output string.

    To continue this issue, I opened squeezer index issue bpo-35855. It describes what I think are the 2 main uses of auto-squeezing and several possible squeezer or related improvements, labeled for easy reference.

    Some existing and new issues related to some of the off-topic messages: bpo-21261: Dict key tab completion. bpo-22121: Start in $HOME. bpo-24776: Configdialog font tab user interface -- add comment. bpo-25522: Save-as warning -- include unimportable names. bpo-28775: Set start-up directory bpo-32761: Modern Mac Keyset. bpo-33397: Change font size with cntl +- (text view first).

    bpo-35763: Calltips: make positional note smaller. bpo-35768: Detecting monospaced fonts with font sample -- automeasure. bpo-35769: ''Untitled" to "untitled" (fixed).