Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

julia-mode.el Indentation for long lists of keyword function arguments #9831

Closed
pwl opened this issue Jan 18, 2015 · 10 comments
Closed

julia-mode.el Indentation for long lists of keyword function arguments #9831

pwl opened this issue Jan 18, 2015 · 10 comments

Comments

@pwl
Copy link
Contributor

pwl commented Jan 18, 2015

I am working with function definitions with long lists of keyword arguments. Surprisingly, after adding even more keyword arguments I can no longer format the argument list properly. This bug seems to depend on the total count of characters enclosed in parenthesis (and is not specific for keyword arguments). For example

function test(;
              a = 1,
              a = 1,
              a = 1,
              a = 1,
              a = 1,
              a = 1,
              auuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuu = 1,
              auuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuu = 1,
              auuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuu = 1,
    a = 1,                      # indentation breaks from now on
    a = 1,
    a = 1,
    a = 1,
    a = 1,
    a = 1,
    a = 1,
    a = 1,
    args...)
end


# the same number of arguments, lower character count
function test(;
              a = 1,
              a = 1,
              a = 1,
              a = 1,
              a = 1,
              a = 1,
              a = 1,
              a = 1,
              a = 1,
              a = 1,
              a = 1,
              a = 1,
              a = 1,
              a = 1,
              a = 1,
              a = 1,
              a = 1,
              args...)
end

# no keyword arguments at all, but indentation is still broken
function test(auuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuu,
              auuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuu,
              auuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuu,
              a,                # indentation breaks from now on
    a,
    a,
    a,
    a,
    a,
    a,
    a)
end

Sorry for all the auuuu's, I should have chosen a better sounding name:-).

@Wilfred
Copy link
Contributor

Wilfred commented Jan 18, 2015

I strongly suspect this is related to #9308. Because of this tradeoff, there will always be a point where a very large number of arguments break indentation. However, I would have expected the failure case to just reuse the previous line's indentation. I'll take a look.

@Wilfred
Copy link
Contributor

Wilfred commented Jan 18, 2015

Yep, this issue can be fixed by increasing the value of julia-max-paren-lookback. I think the proper solution is to benchmark julia-paren-indent and see if we can increase julia-max-paren-lookback.

@pwl
Copy link
Contributor Author

pwl commented Jan 18, 2015

Thanks, I increased the value of julia-max-paren-lookback to 2000, which fixed the issue, and I don't see any noticeable difference in performance. But now I also noticed that in both cases (i.e. 400 and 2000) holding C-m at the end of the file outside of a function causes emacs to hang for a while before adding newlines (although it doesn't seem to be connected to this issue).

@Wilfred
Copy link
Contributor

Wilfred commented Jan 18, 2015

That's exactly the same issue -- C-m will call newline-and-indent, which calls julia-paren-indent. julia-parent-indent is expensive sadly.

@Wilfred
Copy link
Contributor

Wilfred commented Jan 18, 2015

That said, I'm finding performance is still pretty good with higher values of julia-max-paren-lookback. Could you share the file where you're seeing poor performance of C-m?

@pwl
Copy link
Contributor Author

pwl commented Jan 18, 2015

https://github.com/pwl/DASSL.jl/blob/master/src/DASSL.jl

At the start of the file C-m reacts immediately, but at the end (but inside module and outside functions) C-m seems to react slightly slower (but barely noticeably) and when I press and hold C-m to insert multiple lines emacs doesn't react until I release C-m, then after a slight delay the lines are inserted.

@Wilfred
Copy link
Contributor

Wilfred commented Feb 28, 2015

OK, I dug into this a little today. Pressing C-m (or just <RET>) is instant, but holding it is not.

I ran the following test:

  1. I checkout out DASSL.jl at the last commit before your previous comment.
  2. (setq julia-max-paren-lookback 2000)
  3. M-x profiler-start RET cpu RET This is a profiler which samples every millisecond.
  4. Press and hold C-m for a second (approximately).
  5. M-x profiler-report.

This gives:

Function                                                  CPU samples    %
- command-execute                                                5920  95%
 - call-interactively                                            5920  95%
  - apply                                                        5920  95%
   - ad-Advice-call-interactively                                5920  95%
    - #<subr call-interactively>                                 5920  95%
     - newline-and-indent                                        5721  92%
      - indent-according-to-mode                                 3218  51%
       - julia-indent-line                                       3218  51%
        - let*                                                   3218  51%
         - indent-line-to                                        3218  51%
          - or                                                   3218  51%
           - save-excursion                                      3218  51%
            - let                                                3062  49%
             - condition-case                                    3062  49%
              - progn                                            3062  49%
               - +                                               3062  49%
                - julia-last-open-block                          3062  49%
                 - let                                           3062  49%
                  - julia-last-open-block-pos                    3062  49%
                   - save-excursion                              3062  49%
                    - let                                        3062  49%
                     - while                                     3059  49%
                      - setq                                     2890  46%
                       - cond                                    2882  46%
                        - and                                    1934  31%
                         - not                                   1846  29%
                          + julia-in-brackets                    1518  24%
                          + julia-in-comment                      328   5%
                         + equal                                   85   1%
                        - julia-at-keyword                        937  15%
                         + and                                    933  15%
                      + julia-safe-backward-sexp                  159   2%
                      + not                                         7   0%
            + condition-case                                      156   2%
      - newline                                                  2503  40%
       - self-insert-command                                     2503  40%
        - apply                                                  2503  40%
         + ad-Advice-self-insert-command                         2503  40%
     + smex                                                       199   3%
+ ...                                                             274   4%
+ timer-event-handler                                               6   0%

Since we're sampling every millisecond (controlled by profiler-sampling-interval), this took around six seconds. About 40% of that was ad-Advice-self-insert-command which I don't think relates to julia-mode.

Looking at this, I think a simple LRU cache that's cleared when earlier parts of the buffer are modified would really help performance here. We're calling julia-in-brackets over and over for the same positions without buffer changes.

I agree it looks this is independent of julia-max-paren-lookback. Since you found this value was too small on real world code, I think we should also increase the default here.

@yuyichao
Copy link
Contributor

I guess this is not limited to keyword arguments either. Long list, and even functions with too many lines are also broken.

@yuyichao
Copy link
Contributor

This is almost fixed by #15156, however, there seems to be still a limit on the number of non-empty lines in a block.

@yuyichao
Copy link
Contributor

Remaining issue moved to JuliaEditorSupport/julia-emacs#5

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants