Re: [PATCH 1/3] gitk: turn off undo manager in the text widget

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Nov 7, 2016 at 10:57 AM, Markus Hitter <mah@xxxxxxxxxxx> wrote:
> From e965e1deb9747bbc2b40dc2de95afb65aee9f7fd Mon Sep 17 00:00:00 2001
> From: Markus Hitter <mah@xxxxxxxxxxx>
> Date: Sun, 6 Nov 2016 20:38:03 +0100
> Subject: [PATCH 1/3] gitk: turn off undo manager in the text widget
>
> The diff text widget is read-only, so there's zero point in
> building an undo stack. This change reduces memory consumption of
> this widget by about 95%.
>
> Memory usage of the whole program for viewing a reference commit
> before; 579'692'744 bytes, after: 32'724'446 bytes.
>

Wow. Nice find!

> Test procedure:
>
>  - Choose a largish commit and check it out. In this case one with
>    90'802 lines, 5'006'902 bytes.
>
>  - Have a Tcl version with memory debugging enabled. This is,
>    build one with --enable-symbols=mem passed to configure.
>
>  - Instrument Gitk to regularly show a memory dump. E.g. by adding
>    these code lines at the very bottom:
>
>      proc memDump {} {
>          catch {
>              set output [memory info]
>              puts $output
>          }
>
>          after 3000 memDump
>      }
>
>      memDump
>
>  - Start Gitk, it'll load this largish commit into the diff text
>    field automatically (because it's the current commit).
>
>  - Wait until memory consumption levels out and note the numbers.
>
> Note that the numbers reported by [memory info] are much smaller
> than the ones reported in 'top' (1.75 GB vs. 105 MB in this case),
> likely due to all the instrumentation coming with the debug
> version of Tcl.
>

Still, this is definitely the lions share of the memory issue.
Additionally, this fix seems much better overall and does not harm any
other aspects of gitk, because we only read the widget so there is as
you mentioned, zero reason to build an undo stack.

Thanks for taking the extra time to find a proper solution to this! I
think it makes perfect sense.

> Signed-off-by: Markus Hitter <mah@xxxxxxxxxxx>
> ---
>  gitk | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/gitk b/gitk
> index 805a1c7..8654e29 100755
> --- a/gitk
> +++ b/gitk
> @@ -2403,7 +2403,7 @@ proc makewindow {} {
>
>      set ctext .bleft.bottom.ctext
>      text $ctext -background $bgcolor -foreground $fgcolor \
> -       -state disabled -font textfont \
> +       -state disabled -undo 0 -font textfont \
>         -yscrollcommand scrolltext -wrap none \
>         -xscrollcommand ".bleft.bottom.sbhorizontal set"
>      if {$have_tk85} {
> --
> 2.9.3
>

Nice that such a simple change results in a huge gain. I think this
makes perfect sense.

Regards,
Jake



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]