[racket-dev] indentation: the rb tree didn't help, but a hack to stick-to-next-sexp might

That kind of thing makes a lot of sense to me.

I'd probably write the code a little bit differently, having a
function that takes a string and sees if the text starting at
start-pos matches that string instead of having to special case the
numbers 1, 2, 3, 5, and 7. This will also let you just put the call to
forward-match in the second arm of the 'and', so you can avoid the
promise.

And finally, this is the exact *opposite* of a premature optimization!
It's the perfect kind! The kind based on real data and that actually
matters!

Robby

On Mon, Nov 26, 2012 at 12:28 AM, Danny Yoo <dyoo at hashcollision.org> wrote:
> At least, as far as I can tell, it does not perform any better than the
> splay tree.  It's actually about a second slower when indenting
> drracket/private/unit.rkt.  Darn it.
>
>
> However, I did find something that's slightly nutty: the following patch
> appears to greatly improve indentation:
>
>
> https://github.com/dyoo/racket/commit/36670d335d72bb164f3e5dbd97763d69664337a2
>
>
> This crazy code is motivated by the following snippets from the profiler,
> when a profile call is wrapped around tabify-selection:
>
>
> ------------------------------------------------------------------------------------------------------------
>                                   loop [34]
> 0.1%
>                                   get-backward-sexp method in
> ...k/private/racket.rkt:425:2 [28]       99.9%
>  [37] 50648(61.1%)     0(0.0%)  stick-to-next-sexp? method in
> ...k/private/racket.rkt:425:2 ...
>                                   do-forward-match method in
> ...rk/private/color.rkt:71:2 [50]         99.9%
>
> ...
>
> ------------------------------------------------------------------------------------------------------------
>                                   get-forward-sexp method in
> ...k/private/racket.rkt:425:2 [38]        17.1%
>                                   stick-to-next-sexp? method in
> ...k/private/racket.rkt:425:2 [37]     82.9%
>  [50] 61043(73.6%)    53(0.1%)  do-forward-match method in
> ...rk/private/color.rkt:71:2 ...
>                                   colorer-driver method in
> ...rk/private/color.rkt:71:2 [66]           99.8%
>                                   match-forward method in paren-tree% [72]
> 0.1%
> ------------------------------------------------------------------------------------------------------------
>
>
> If I'm reading the profiler output correctly, this is saying that 61% of the
> time is being spent in stick-to-next-sexp?, and that in stick-to-next-sexp?,
> the majority of the time goes through do-forward-match.  In the diff above,
> I made the code speculatively match the text: if it fails, there's no need
> to call do-forward-match.  I reasoned that it looked expensive to call:
> maybe we can avoid it in the common case.
>
> I'm seeing indentation times cut by 50%; I've been staring at this all day,
> so I don't trust myself yet.  Can anyone else confirm that that they see a
> similarly dramatic improvement in tabification when this hack is in place?
>
>
> I have not committed this yet because it's kludgy code.  :)  I know the
> above is not quite the right way to solve the problem, but it seems like it
> would be worthwhile to do something like this.  Of course, it would be
> better to fix forward-match so it isn't so expensive.
>
> _________________________
>   Racket Developers list:
>   http://lists.racket-lang.org/dev
>