[bug#66796] lint: Speed up the formatting linter.

By storing the bytes to seek to for the start of each line the first time you
want to check a package in a file, rather than figuring out where the package
starts each time.

This cuts down the time to run guix lint -c formatting from 450 seconds to 13
seconds.

* guix/lint.scm (report-formatting-issues): If %check-formatting-seek-lookup
is a hash table, store vlist's in it to map from a line number to a byte to
seek to.
(%check-formatting-seek-lookup): New parameter.
* guix/scripts/lint.scm (guix-lint): Enable faster formatting linting, when
linting all packages.

Change-Id: I34e4d3acfbb1e14e026d2e7f712ba8d22b56c147
---
 guix/lint.scm         | 44 ++++++++++++++++++++++++++++++++++++++++++-
 guix/scripts/lint.scm |  3 +++
 2 files changed, 46 insertions(+), 1 deletion(-)


base-commit: c3cf04d05b452fee549bb84b323d056fd30cef45

Hi,

Christopher Baines <mail@cbaines.net> skribis:

> By storing the bytes to seek to for the start of each line the first time you
> want to check a package in a file, rather than figuring out where the package
> starts each time.
>
> This cuts down the time to run guix lint -c formatting from 450 seconds to 13
> seconds.

Excellent!

> +  (define (seek-to-line port line)
> +    (let ((offset
> +           (vlist-ref
> +            (or (hash-ref (%check-formatting-seek-lookup) file)
> +                (call-with-input-file file
> +                  (lambda (port)
> +                    (let* ((buf-length 80)
> +                           (buf (make-string buf-length)))
> +                      (let loop ((byte-lookup-list '(0)))
> +                        (let* ((rv (%read-delimited! "\n" buf #t port))
> +                               (terminator (car rv))
> +                               (nchars (cdr rv)))
> +                          (cond
> +                           ((eof-object? terminator)
> +                            (let ((byte-lookup-vlist
> +                                   (list->vlist
> +                                    (reverse byte-lookup-list))))
> +                              (hash-set! (%check-formatting-seek-lookup)
> +                                         file
> +                                         byte-lookup-vlist)
> +                              byte-lookup-vlist))
> +
> +                           ((not terminator)
> +                            (loop byte-lookup-list))
> +
> +                           (nchars
> +                            (loop (cons
> +                                   (ftell port)
> +                                   byte-lookup-list))))))))))
> +            (- line 1))))
> +      (set-port-line! port line)
> +      (seek port offset SEEK_SET)
> +      line))

Two things: (1) it’s a bit hard to read, in part due to long
identifiers, and (2) it would be nice to keep this facility orthogonal,
outside the checker.

As it turns out, a similar facility is available in (guix utils),
exposed via ‘go-to-location’.  Would it be possible to use it here?

> +      (let loop ((line-number
> +                  (if (%check-formatting-seek-lookup)
> +                      (seek-to-line port starting-line)
> +                      1))

Answering myself: I guess ‘seek-to-line’ can be replaced by
(go-to-location port starting-line 0).

Thanks!

Ludo’.

Ludovic Courtès <ludo@gnu.org> writes:

> Hi,
>
> Christopher Baines <mail@cbaines.net> skribis:
>
>> By storing the bytes to seek to for the start of each line the first time you
>> want to check a package in a file, rather than figuring out where the package
>> starts each time.
>>
>> This cuts down the time to run guix lint -c formatting from 450 seconds to 13
>> seconds.
>
> Excellent!
>
>> +  (define (seek-to-line port line)
>> +    (let ((offset
>> +           (vlist-ref
>> +            (or (hash-ref (%check-formatting-seek-lookup) file)
>> +                (call-with-input-file file
>> +                  (lambda (port)
>> +                    (let* ((buf-length 80)
>> +                           (buf (make-string buf-length)))
>> +                      (let loop ((byte-lookup-list '(0)))
>> +                        (let* ((rv (%read-delimited! "\n" buf #t port))
>> +                               (terminator (car rv))
>> +                               (nchars (cdr rv)))
>> +                          (cond
>> +                           ((eof-object? terminator)
>> +                            (let ((byte-lookup-vlist
>> +                                   (list->vlist
>> +                                    (reverse byte-lookup-list))))
>> +                              (hash-set! (%check-formatting-seek-lookup)
>> +                                         file
>> +                                         byte-lookup-vlist)
>> +                              byte-lookup-vlist))
>> +
>> +                           ((not terminator)
>> +                            (loop byte-lookup-list))
>> +
>> +                           (nchars
>> +                            (loop (cons
>> +                                   (ftell port)
>> +                                   byte-lookup-list))))))))))
>> +            (- line 1))))
>> +      (set-port-line! port line)
>> +      (seek port offset SEEK_SET)
>> +      line))
>
> Two things: (1) it’s a bit hard to read, in part due to long
> identifiers, and (2) it would be nice to keep this facility orthogonal,
> outside the checker.
>
> As it turns out, a similar facility is available in (guix utils),
> exposed via ‘go-to-location’.  Would it be possible to use it here?
>
>> +      (let loop ((line-number
>> +                  (if (%check-formatting-seek-lookup)
>> +                      (seek-to-line port starting-line)
>> +                      1))
>
> Answering myself: I guess ‘seek-to-line’ can be replaced by
> (go-to-location port starting-line 0).

Cool, this simplifies the change a lot, I've pushed the amended version
to master as aa98a976079101318d53b412fef6c722bb4332c9.

Thanks,

Chris