Patchwork [libfortran,4.7/4.8/4.9,Regression] PR38199 missed optimization: I/O performance

login
register
mail settings
Submitter jerry DeLisle
Date March 8, 2014, 8:13 p.m.
Message ID <531B79F8.2090403@charter.net>
Download mbox | patch
Permalink /patch/328242/
State New
Headers show

Comments

jerry DeLisle - March 8, 2014, 8:13 p.m.
On 03/08/2014 04:58 AM, Steven Bosscher wrote:
> On Sat, Mar 8, 2014 at 7:38 AM, Jerry DeLisle wrote:
>> The speedup is accomplished by simply skipping over spaces without calling
>> next_read, then backing up one character and letting the existing execution path
>> proceed, preserving all the end of record code needed in next_char.
>>
>> I also remove some unneeded error checks.
> 
> Would it be enough to make them "unlikely" instead?
> 
> -      if (length < 0)
> +      if (unlikely(length < 0))
> 

Here is the revised patch leaving the error checks in place and using unlikely().

I have also added handling of kind=4 character arrays.

Regression tested on x86-64.

OK for trunk?

Jerry
Tobias Burnus - March 8, 2014, 10:22 p.m.
Jerry DeLisle wrote:
> Here is the revised patch leaving the error checks in place and using 
> unlikely(). I have also added handling of kind=4 character arrays. 
> Regression tested on x86-64. OK for trunk?

OK. Minor nit:

+      if ( unlikely(length < 0))


The space shall be after unlikely not before.

Tobias

Patch

Index: list_read.c
===================================================================
--- list_read.c	(revision 208303)
+++ list_read.c	(working copy)
@@ -160,7 +160,7 @@  next_char (st_parameter_dt *dtp)
 
       dtp->u.p.line_buffer_pos = 0;
       dtp->u.p.line_buffer_enabled = 0;
-    }    
+    }
 
   /* Handle the end-of-record and end-of-file conditions for
      internal array unit.  */
@@ -208,16 +208,16 @@  next_char (st_parameter_dt *dtp)
          c = cc;
        }
 
-      if (length < 0)
+      if ( unlikely(length < 0))
 	{
 	  generate_error (&dtp->common, LIBERROR_OS, NULL);
 	  return '\0';
 	}
-  
+
       if (is_array_io (dtp))
 	{
 	  /* Check whether we hit EOF.  */ 
-	  if (length == 0)
+	  if (unlikely (length == 0))
 	    {
 	      generate_error (&dtp->common, LIBERROR_INTERNAL_UNIT, NULL);
 	      return '\0';
@@ -264,6 +264,48 @@  eat_spaces (st_parameter_dt *dtp)
 {
   int c;
 
+  /* If internal character array IO, peak ahead and seek past spaces.
+     This is an optimazation to eliminate numerous calls to
+     next character unique to character arrays with large character
+     lengths (PR38199). */
+  if (is_array_io (dtp))
+    {
+      gfc_offset offset = stell (dtp->u.p.current_unit->s);
+      gfc_offset limit = dtp->u.p.current_unit->bytes_left;
+
+      if (dtp->common.unit) /* kind=4 */
+	{
+	  gfc_char4_t cc;
+	  limit *= (sizeof (gfc_char4_t));
+	  do
+	    {
+	      cc = dtp->internal_unit[offset];
+	      offset += (sizeof (gfc_char4_t));
+	      dtp->u.p.current_unit->bytes_left--;
+	    }
+	  while (offset < limit && (cc == (gfc_char4_t)' '
+		  || cc == (gfc_char4_t)'\t'));
+	  /* Back up, seek ahead, and fall through to complete the
+	     process so that END conditions are handled correctly.  */
+	  dtp->u.p.current_unit->bytes_left++;
+	  sseek (dtp->u.p.current_unit->s,
+		  offset-(sizeof (gfc_char4_t)), SEEK_SET);
+	}
+      else
+	{
+	  do
+	    {
+	      c = dtp->internal_unit[offset++];
+	      dtp->u.p.current_unit->bytes_left--;
+	    }
+	  while (offset < limit && (c == ' ' || c == '\t'));
+	  /* Back up, seek ahead, and fall through to complete the
+	     process so that END conditions are handled correctly.  */
+	  dtp->u.p.current_unit->bytes_left++;
+	  sseek (dtp->u.p.current_unit->s, offset-1, SEEK_SET);
+	}
+    }
+  /* Now skip spaces, EOF and EOL are handled in next_char.  */
   do
     c = next_char (dtp);
   while (c != EOF && (c == ' ' || c == '\t'));