On Wed, Jun 29, 2016 at 12:30:50PM +0200, Marc Lehmann wrote:
> On Tue, Jun 28, 2016 at 03:24:45PM +0800, Fengguang Wu
> <[email protected]> wrote:
> > > - Is the speed gain given by hashing significant enough to justify the
> > > additional space of 'rgb24_index'?
> >
> > There will no user noticeable performance change, I guess. Feel free
>
> Do you happen to have some benchmark numbers assuming excessive use of
> this sequence? If it would considerably reduce cpu usage of urxvt it would
> be useful, even if there is no obvious user-visible difference.
>
> I guess a difficult workload would be something like libcaca using these
> sequences, but likewise, I am not sure it is worth optimizing for that.
>
> In any case, for me the patch is good to go, but Emmanuele is not convinced
> either way about the hashing. If it provides no real difference in practial
> situations, he will remove it when applying. IF it does make an actual
> difference, we would like to keep it.
I tried removing the rgb24_index[] code in map_rgb24_color():
--- a/src/command.C
+++ b/src/command.C
@@ -3382,10 +3382,6 @@ rxvt_term::map_rgb24_color (unsigned int r, unsigned int
g, unsigned int b)
(b >> (8 - RGB24_HASH_BITS))) & (RGB24_HASH_SIZE - 1);
assert (key < RGB24_HASH_SIZE);
- idx = rgb24_index[key];
- if (idx && rgb24_color[idx] == color)
- return idx + minTermCOLOR24;
-
idx_r = r / (0xff / (Red_levels - 1));
idx_g = g / (0xff / (Green_levels - 1));
idx_b = b / (0xff / (Blue_levels - 1));
@@ -3450,7 +3446,6 @@ rxvt_term::map_rgb24_color (unsigned int r, unsigned int
g, unsigned int b)
#endif
update:
- rgb24_index[key] = idx;
rgb24_color[idx] = color;
rgb24_seqno[idx] = ++rgb24_sequence;
And do notice slightly increased total time:
w/ rgb24_index[]
( for i in {1..100}; do; 24-bit-color.sh; done; ) 2.62s user 2.02s system 31%
cpu 14.674 total
( for i in {1..100}; do; 24-bit-color.sh; done; ) 2.64s user 2.12s system 31%
cpu 14.933 total
( for i in {1..100}; do; 24-bit-color.sh; done; ) 2.67s user 2.08s system 31%
cpu 14.870 total
w/o rgb24_index[]
( for i in {1..100}; do; 24-bit-color.sh; done; ) 2.56s user 2.16s system 31%
cpu 14.817 total
( for i in {1..100}; do; 24-bit-color.sh; done; ) 2.57s user 2.28s system 32%
cpu 15.168 total
( for i in {1..100}; do; 24-bit-color.sh; done; ) 2.52s user 2.42s system 31%
cpu 15.472 total
It looks the more rgb24_index[] lookups increased cacheline footprint
of urxvt, which in turn increased system time.
However that should be the worst case. In light use of 24-bit colors
(rgb24_color[idx] == color) will likely be true and there will be no
further rgb24_color[] lookup.
Thanks,
Fengguang
_______________________________________________
rxvt-unicode mailing list
[email protected]
http://lists.schmorp.de/mailman/listinfo/rxvt-unicode
