beyondgrep / ack2

**ack 2 is no longer being maintained. ack 3 is the latest version.**
https://github.com/beyondgrep/ack3/
Other
1.48k stars 140 forks source link

Reworked context handling to be faster #503

Closed sth closed 10 years ago

sth commented 10 years ago

I did some changes to the context handling in ack to speed up searches with -A, -B and -C (see #487):

Additionally I moved most of the context handling logic into a few specialized functions, setup_line_context(), setup_line_context_for_file() and print_line_if_context(), making the rest of the code simpler.

The diff is larger than I'd like, but the resulting code should be clearer and easier to understand. The tests pass. Speed, while not yet optimal, is significantly improved:

                                           |   1.96 |   2.12 |   2.14 |   HEAD
------------------------------------------------------------------------------
            ack int /home/sth/work/devices |   1.25 |   1.47 |   0.86 |   0.88
    ack std::vector /home/sth/work/devices |   0.50 |   1.20 |   0.37 |   0.34
         ack xyxyxy /home/sth/work/devices |   0.46 |   1.23 |   0.37 |   0.35
        ack int -A3 /home/sth/work/devices |   1.71 |   3.48 |   3.02 |   2.18
ack std::vector -A3 /home/sth/work/devices |   0.56 |   2.80 |   2.51 |   1.78
     ack xyxyxy -A3 /home/sth/work/devices |   0.49 |   2.90 |   2.53 |   1.78
        ack int -B3 /home/sth/work/devices |   2.03 |   3.79 |   3.28 |   2.50
ack std::vector -B3 /home/sth/work/devices |   0.74 |   2.98 |   2.75 |   2.11
     ack xyxyxy -B3 /home/sth/work/devices |   0.65 |   3.14 |   2.77 |   2.15
        ack int -C1 /home/sth/work/devices |   1.94 |   4.14 |   3.62 |   2.38
ack std::vector -C1 /home/sth/work/devices |   0.70 |   3.36 |   3.21 |   2.10
     ack xyxyxy -C1 /home/sth/work/devices |   0.67 |   3.52 |   3.16 |   1.97
        ack int -C5 /home/sth/work/devices |   2.41 |   4.47 |   3.98 |   2.67
ack std::vector -C5 /home/sth/work/devices |   0.73 |   3.34 |   3.14 |   2.18
     ack xyxyxy -C5 /home/sth/work/devices |   0.67 |   3.41 |   3.04 |   2.07

(In this table 2.14 is the current head of the dev branch)

hoelzro commented 10 years ago

Nice! I'll look over this later.

petdance commented 10 years ago

My top concern here is that improvements in -A/-B/-C don't degrade performance elsewhere.

sth commented 10 years ago

Normal, non-context searches should be basically not affected at all by these changes. Nearly all changes are not in the main code path, but inside if blocks that are only entered when a context search is done (if ($n_before_ctx_lines || $n_after_ctx_lines) in current ack or if ($is_tracking_context) with the changes of this PR).

There was one addtional function call per searched file and some unnecessary work that was done when --lines was specified, but I moved that code also into if ($is_tracking_context) blocks. Now these cases are also only executed when necessary and should cause basically no overhead otherwise.

For these checks for to work I had to make $is_tracking_context a global variable. Maybe this could also be done better differently, but I'm not sure how.

hoelzro commented 10 years ago

@sth I haven't looked much, but I like what I've seen so far. I'm not crazy about the $is_tracking_context global, but if it's unavoidable, that's how it is. I'll see about merging this in tomorrow.

hoelzro commented 10 years ago

Everything looks good to me here!