Re: Enable accessibility by default in development releases?

["Padraig O'Briain" <Padraig Obriain Sun COM>]

The problem seems to be that the function vte_terminal_process_incoming
causes the "text-inserted" signal to be emitted for what seems like
every character.

The attached patch cauwes about 6% degradation when cat'ing a large file
in gnome-terminal with accessibility enabled.

I would be useful to know whether it has any impact on the accessibility
of gnome-terminal.

Padraig

On Wed, 2005-11-23 at 09:54 +0000, Padraig O'Briain wrote:
> I have looked a bit closer and I see that the function
> vte_terminal_accessible_text_modified would be called over 3000 times a
> second if it was not doing so much work. Need to figure out why.
> 
> Padraig
> 
> On Wed, 2005-11-23 at 09:23 +0000, Padraig O'Briain wrote:
> > It looks to me that display_timeout is called 50 times a second; this
> > function calls vte_terminal_process_incoming which emits "text-inserted"
> > signal which causes the accessibility layer to refetch the contents of
> > the terminal.
> > 
> > I do not expect Kjartan's patch to fix the problem but I would like to
> > see what difference, if any, it makes.
> > 
> > It looks like we need to be more clever in processing the
> > "text-inserted" events.
> > 
> > Padraig
> > 
> > On Tue, 2005-11-22 at 21:38 +0000, John Rice wrote:
> > > Looking at Ross's suggestion and your profile data Padraig, does look
> > > as if something like the "scrollbar-updated signal is causing the a11y
> > > layer to refetch the contents of the terminal several times on every
> > > output"
> > > 
> > > So Kjartan's patch replaces the following g_return_if_fail macros in
> > > vteaccess.c with g_asserts allowing them to be compiled out which is
> > > great, but I fear the % of cpu time in the function they take up will
> > > be small relative to the  time taken to update the snap_shot
> > > text :( But certainly worth trying. Suspect we'll get most mileage out
> > > of looking how to batch the scroll events, be clever on the type of
> > > redraw we are doing ...
> > > 
> > > JR
> > > 
> > > /vte-0.11.15/src/vteaccess.c
> > >     249 static void
> > >     250 vte_terminal_accessible_update_private_data_if_needed(AtkObject *text,
> > >     251 						      char **old, glong *olen)
> > >     252 {
> > > 
> > > 		:
> > >     259 
> > >     260 	g_return_if_fail(VTE_IS_TERMINAL_ACCESSIBLE(text));
> > >     261 
> > >     262 	/* Retrieve the private data structure.  It must already exist. */
> > >     263 	priv = g_object_get_data(G_OBJECT(text),
> > >     264 				 VTE_TERMINAL_ACCESSIBLE_PRIVATE_DATA);
> > >     265 	g_return_if_fail(priv != NULL);
> > >     266 
> > >     267 	/* If nothing's changed, just return immediately. */
> > >     268 	if ((priv->snapshot_contents_invalid == FALSE) &&
> > >     269 	    (priv->snapshot_caret_invalid == FALSE)) {
> > >     270 		if (old) {
> > >     271 			if (priv->snapshot_text) {
> > >     272 				*old = g_malloc(priv->snapshot_text->len + 1);
> > >     273 				memcpy(*old,
> > >     274 				       priv->snapshot_text->str,
> > >     275 				       priv->snapshot_text->len);
> > > 
> > > 
> > > 
> > > 
> > > Padraig O'Briain wrote: 
> > > > I have looked at profile data when accessibility is enabled and most of
> > > > the time is spent in
> > > > vte_terminal_accessible_update_private_data_if_needed.
> > > > 
> > > > I need to apply the patch to replace g_return_if_fail calls with
> > > > g_assert calls and see what improvement that makes.
> > > > 
> > > > Padraig
> > > > 
> > > > On Tue, 2005-11-22 at 12:16 +0000, John Rice wrote:
> > > >   
> > > > > As can sometimes be the case with automated benchmarks, not quite what
> > > > > it seems :(
> > > > > 
> > > > > Derek had an issue with doing one of the automated benchmarks in
> > > > > gnome-terminal and gedit, so he went back and rechecked them all
> > > > > manually. Difference from previous automated results below, when
> > > > > cat'ing a large file in GT or opening a large file in GEdit.
> > > > > 
> > > > > Looks like if you need to cat large files flick the off button :)
> > > > > Apart from that things look ok for enabling this by default. All the
> > > > > previous automated benchmark tests were uneffected.
> > > > > 
> > > > > JR
> > > > > Revised results from manually measuring;
> > > > > 
> > > > > o cat a big file in gnome-terminal (Using a .html file, 26,332,850 bytes)
> > > > > -  AllY Off: 40 secs
> > > > > -  AllY On: 214 secs  (over 500% increase in mean scroll time)
> > > > > 
> > > > > o open a file in gedit (same .html file as above)
> > > > > -  AllY Off: 3.18 sec
> > > > > -  AllY On:  4.49 sec  (40% increase in mean file load time)
> > > > > 
> > > > > 
> > > > > 
> > > > > Federico Mena Quintero wrote: 
> > > > >     
> > > > > > On Mon, 2005-11-21 at 16:34 +0000, John Rice wrote:
> > > > > > 
> > > > > >   
> > > > > >       
> > > > > > > Doing 'cat' of a big file in gnome-terminal was the only area where a
> > > > > > > significant difference was seen. Approx 45 secs with A11Y ON, and 42.4
> > > > > > > secs with A11Y Off.
> > > > > > > 
> > > > > > > All other areas looked at above were not significantly different
> > > > > > > comparing A11Y On Vs A11Y Off.
> > > > > > >     
> > > > > > >         
> > > > > > This is great news :)  And the difference in gnome-terminal could be
> > > > > > just noise.
> > > > > > 
> > > > > > So, have people *felt* a performance impact from running their session
> > > > > > with a11y enabled, or is it just a placebo?
> > > > > > 
> > > > > > Anyway, let's go ahead with the change - this will let us test the code
> > > > > > better.
> > > > > > 
> > > > > >   Federico
> > > > > > 
> > > > > > _______________________________________________
> > > > > > desktop-devel-list mailing list
> > > > > > desktop-devel-list gnome org
> > > > > > http://mail.gnome.org/mailman/listinfo/desktop-devel-list
> > > > > >   
> > > > > >       
> > > > > _______________________________________________
> > > > > desktop-devel-list mailing list
> > > > > desktop-devel-list gnome org
> > > > > http://mail.gnome.org/mailman/listinfo/desktop-devel-list
> > > > >     
> > > > 
> > > >   
> > > 
> > > _______________________________________________
> > > desktop-devel-list mailing list
> > > desktop-devel-list gnome org
> > > http://mail.gnome.org/mailman/listinfo/desktop-devel-list
> > 
> > _______________________________________________
> > desktop-devel-list mailing list
> > desktop-devel-list gnome org
> > http://mail.gnome.org/mailman/listinfo/desktop-devel-list
> 
> _______________________________________________
> desktop-devel-list mailing list
> desktop-devel-list gnome org
> http://mail.gnome.org/mailman/listinfo/desktop-devel-list

*** vte.c.ORIG	Wed Nov 23 11:08:12 2005
--- vte.c	Wed Nov 23 11:10:47 2005
***************
*** 7601,7608 ****
  				vte_terminal_ensure_cursor(terminal, FALSE);
  				inserted = FALSE;
  			}
- 			/* Flush any pending "inserted" signals. */
- 			vte_terminal_emit_pending_text_signals(terminal, quark);
  			/* Call the right sequence handler for the requested
  			 * behavior. */
  			again = vte_terminal_handle_sequence(GTK_WIDGET(terminal),
--- 7601,7606 ----
***************
*** 7611,7618 ****
  							     params);
  			/* Skip over the proper number of unicode chars. */
  			start = (next - wbuf);
- 			/* Flush any pending signals. */
- 			vte_terminal_emit_pending_text_signals(terminal, quark);
  			modified = TRUE;
  		} else
  		/* Second, we have a NULL match, and next points to the very
--- 7609,7614 ----