Re: [Mono-dev] [Patch] to significantly shorten Char.cs

2008-07-08 Thread Rodrigo Kumpera 
Hi Andreas,

Sorry for the very long wait, I could only review your patch this week. The
Is* patch is correct and indeed increase performance, please commit it.

Thanks for the contribution,
Rodrigo


On Sun, Jun 29, 2008 at 7:37 PM, Andreas Nahr 
[EMAIL PROTECTED] wrote:

  Hi Rodrigo,



 very good suggestion ;)

 Attached is a patch that only contains the Is* changes.



 Happy Hacking

 Andreas



 *Von:* [EMAIL PROTECTED] [mailto:
 [EMAIL PROTECTED] *Im Auftrag von *Rodrigo Kumpera
 *Gesendet:* Samstag, 28. Juni 2008 00:37
 *An:* Andreas Nahr
 *Cc:* mono-devel

 *Betreff:* Re: [Mono-dev] [Patch] to significantly shorten Char.cs



 Andreas,

 How about splitting your patches in two? One with the lesser risky
 CheckParameter changes and other with the Is*** changes.
 The CheckParameter changes look lovely, go ahead commit then.
 I would like to do some testing on other changes, that are more comple and
 unfortunately I won't have time until late next week.

 I know it's more work on your side, but it makes the review process easier
 for both of us.

 Thanks,
 Rodrigo

  On Thu, Jun 26, 2008 at 6:24 PM, Andreas Nahr 
 [EMAIL PROTECTED] wrote:

 Well the main improvement is the shortening of the String-Parameter
 methods.
 It does not affect performance negatively as far as I measured because the
 methods now get inlined but were too large before.
 I know that the code wasn't duplicated for performance reasons before,
 because I actually wrote that code. Well most of Char anyways... ;)

 IsWhiteSpace is now somewhat faster. Similar Prog from below (do not have
 the original anymore) went from 21 sec total to 16,5 sec (AMD 3,2Ghz XP)
 (testing one positive + one negative case). It simply saves some opcodes.

 public static void Main()
 {
int tickCount = Environment.TickCount;
for (int i = 0; i  0x3b9aca00; i++)
{
char.IsWhiteSpace('v');
}
Console.WriteLine((int) (Environment.TickCount - tickCount));
tickCount = Environment.TickCount;
for (int j = 0; j  0x3b9aca00; j++)
{
char.IsWhiteSpace(' ');
}
Console.WriteLine((int) (Environment.TickCount - tickCount));
 }

 And no, IsWhiteSpace did not get optimized away ;)

  -Ursprüngliche Nachricht-
  Von: [EMAIL PROTECTED] [mailto:mono-devel-list-
  [EMAIL PROTECTED] Im Auftrag von Atsushi Eno
  Gesendet: Donnerstag, 26. Juni 2008 23:25
  An: 'mono-devel'
  Betreff: Re: [Mono-dev] [Patch] to significantly shorten Char.cs

 
  I have some comments even without reading your application/octet-stream
  patch (I ususally don't read them).
 
  - Sometimes we have some code pieces that look like duplicates
 exactly because of performance improvements.
  - For performance improvements there should be some benchmark
 results with sources.
 
  Atsushi Eno
 
 
  Andreas Nahr wrote:
   The attached patch reduces code duplication in Char by more than 130
  lines
   and improves the performance of some methods a little bit.
  
   Somebody please have a short look...
  
   Happy Hacking
   Andreas
  
  
   -
  ---
  
   ___
   Mono-devel-list mailing list
   Mono-devel-list@lists.ximian.com
   http://lists.ximian.com/mailman/listinfo/mono-devel-list
 
  ___
  Mono-devel-list mailing list
  Mono-devel-list@lists.ximian.com
  http://lists.ximian.com/mailman/listinfo/mono-devel-list

 ___
 Mono-devel-list mailing list
 Mono-devel-list@lists.ximian.com
 http://lists.ximian.com/mailman/listinfo/mono-devel-list



___
Mono-devel-list mailing list
Mono-devel-list@lists.ximian.com
http://lists.ximian.com/mailman/listinfo/mono-devel-list

Re: [Mono-dev] [Patch] to significantly shorten Char.cs

2008-06-29 Thread Andreas Nahr 
Hi Rodrigo,

 

very good suggestion ;)

Attached is a patch that only contains the Is* changes.

 

Happy Hacking

Andreas

 

Von: [EMAIL PROTECTED]
[mailto:[EMAIL PROTECTED] Im Auftrag von Rodrigo
Kumpera
Gesendet: Samstag, 28. Juni 2008 00:37
An: Andreas Nahr
Cc: mono-devel
Betreff: Re: [Mono-dev] [Patch] to significantly shorten Char.cs

 

Andreas,

How about splitting your patches in two? One with the lesser risky
CheckParameter changes and other with the Is*** changes.
The CheckParameter changes look lovely, go ahead commit then.
I would like to do some testing on other changes, that are more comple and
unfortunately I won't have time until late next week.

I know it's more work on your side, but it makes the review process easier
for both of us. 

Thanks,
Rodrigo



On Thu, Jun 26, 2008 at 6:24 PM, Andreas Nahr
[EMAIL PROTECTED] wrote:

Well the main improvement is the shortening of the String-Parameter methods.
It does not affect performance negatively as far as I measured because the
methods now get inlined but were too large before.
I know that the code wasn't duplicated for performance reasons before,
because I actually wrote that code. Well most of Char anyways... ;)

IsWhiteSpace is now somewhat faster. Similar Prog from below (do not have
the original anymore) went from 21 sec total to 16,5 sec (AMD 3,2Ghz XP)
(testing one positive + one negative case). It simply saves some opcodes.

public static void Main()
{
   int tickCount = Environment.TickCount;
   for (int i = 0; i  0x3b9aca00; i++)
   {
   char.IsWhiteSpace('v');
   }
   Console.WriteLine((int) (Environment.TickCount - tickCount));
   tickCount = Environment.TickCount;
   for (int j = 0; j  0x3b9aca00; j++)
   {
   char.IsWhiteSpace(' ');
   }
   Console.WriteLine((int) (Environment.TickCount - tickCount));
}

And no, IsWhiteSpace did not get optimized away ;)

 -Ursprüngliche Nachricht-
 Von: [EMAIL PROTECTED] [mailto:mono-devel-list-
 [EMAIL PROTECTED] Im Auftrag von Atsushi Eno
 Gesendet: Donnerstag, 26. Juni 2008 23:25
 An: 'mono-devel'
 Betreff: Re: [Mono-dev] [Patch] to significantly shorten Char.cs


 I have some comments even without reading your application/octet-stream
 patch (I ususally don't read them).

 - Sometimes we have some code pieces that look like duplicates
exactly because of performance improvements.
 - For performance improvements there should be some benchmark
results with sources.

 Atsushi Eno


 Andreas Nahr wrote:
  The attached patch reduces code duplication in Char by more than 130
 lines
  and improves the performance of some methods a little bit.
 
  Somebody please have a short look...
 
  Happy Hacking
  Andreas
 
 
  -
 ---
 
  ___
  Mono-devel-list mailing list
  Mono-devel-list@lists.ximian.com
  http://lists.ximian.com/mailman/listinfo/mono-devel-list

 ___
 Mono-devel-list mailing list
 Mono-devel-list@lists.ximian.com
 http://lists.ximian.com/mailman/listinfo/mono-devel-list

___
Mono-devel-list mailing list
Mono-devel-list@lists.ximian.com
http://lists.ximian.com/mailman/listinfo/mono-devel-list

 



Char.patch
Description: Binary data
___
Mono-devel-list mailing list
Mono-devel-list@lists.ximian.com
http://lists.ximian.com/mailman/listinfo/mono-devel-list

Re: [Mono-dev] [Patch] to significantly shorten Char.cs

2008-06-27 Thread Rodrigo Kumpera 
Andreas,

How about splitting your patches in two? One with the lesser risky
CheckParameter changes and other with the Is*** changes.
The CheckParameter changes look lovely, go ahead commit then.
I would like to do some testing on other changes, that are more comple and
unfortunately I won't have time until late next week.

I know it's more work on your side, but it makes the review process easier
for both of us.

Thanks,
Rodrigo


On Thu, Jun 26, 2008 at 6:24 PM, Andreas Nahr 
[EMAIL PROTECTED] wrote:

 Well the main improvement is the shortening of the String-Parameter
 methods.
 It does not affect performance negatively as far as I measured because the
 methods now get inlined but were too large before.
 I know that the code wasn't duplicated for performance reasons before,
 because I actually wrote that code. Well most of Char anyways... ;)

 IsWhiteSpace is now somewhat faster. Similar Prog from below (do not have
 the original anymore) went from 21 sec total to 16,5 sec (AMD 3,2Ghz XP)
 (testing one positive + one negative case). It simply saves some opcodes.

 public static void Main()
 {
int tickCount = Environment.TickCount;
for (int i = 0; i  0x3b9aca00; i++)
{
char.IsWhiteSpace('v');
}
Console.WriteLine((int) (Environment.TickCount - tickCount));
tickCount = Environment.TickCount;
for (int j = 0; j  0x3b9aca00; j++)
{
char.IsWhiteSpace(' ');
}
Console.WriteLine((int) (Environment.TickCount - tickCount));
 }

 And no, IsWhiteSpace did not get optimized away ;)

  -Ursprüngliche Nachricht-
  Von: [EMAIL PROTECTED] [mailto:mono-devel-list-
  [EMAIL PROTECTED] Im Auftrag von Atsushi Eno
  Gesendet: Donnerstag, 26. Juni 2008 23:25
  An: 'mono-devel'
  Betreff: Re: [Mono-dev] [Patch] to significantly shorten Char.cs
 
  I have some comments even without reading your application/octet-stream
  patch (I ususally don't read them).
 
  - Sometimes we have some code pieces that look like duplicates
 exactly because of performance improvements.
  - For performance improvements there should be some benchmark
 results with sources.
 
  Atsushi Eno
 
 
  Andreas Nahr wrote:
   The attached patch reduces code duplication in Char by more than 130
  lines
   and improves the performance of some methods a little bit.
  
   Somebody please have a short look...
  
   Happy Hacking
   Andreas
  
  
   -
  ---
  
   ___
   Mono-devel-list mailing list
   Mono-devel-list@lists.ximian.com
   http://lists.ximian.com/mailman/listinfo/mono-devel-list
 
  ___
  Mono-devel-list mailing list
  Mono-devel-list@lists.ximian.com
  http://lists.ximian.com/mailman/listinfo/mono-devel-list

 ___
 Mono-devel-list mailing list
 Mono-devel-list@lists.ximian.com
 http://lists.ximian.com/mailman/listinfo/mono-devel-list

___
Mono-devel-list mailing list
Mono-devel-list@lists.ximian.com
http://lists.ximian.com/mailman/listinfo/mono-devel-list

Re: [Mono-dev] [Patch] to significantly shorten Char.cs

2008-06-26 Thread Atsushi Eno 
I have some comments even without reading your application/octet-stream
patch (I ususally don't read them).

- Sometimes we have some code pieces that look like duplicates
   exactly because of performance improvements.
- For performance improvements there should be some benchmark
   results with sources.

Atsushi Eno


Andreas Nahr wrote:
 The attached patch reduces code duplication in Char by more than 130 lines
 and improves the performance of some methods a little bit.
 
 Somebody please have a short look...
 
 Happy Hacking
 Andreas
 
 
 
 
 ___
 Mono-devel-list mailing list
 Mono-devel-list@lists.ximian.com
 http://lists.ximian.com/mailman/listinfo/mono-devel-list

___
Mono-devel-list mailing list
Mono-devel-list@lists.ximian.com
http://lists.ximian.com/mailman/listinfo/mono-devel-list

Re: [Mono-dev] [Patch] to significantly shorten Char.cs

2008-06-26 Thread Rodrigo Kumpera 
Your patch have a huge ammount of useless whitespace changes, these make
reviews a lot hard.

Please provide some benchmarks so we can run it locally and see the
performance improvements.

I would say that moving the argumento checks to a funcion is a bad idea. We
should move the exception
raising part, thou.



2008/6/26 Andreas Nahr [EMAIL PROTECTED]:

 The attached patch reduces code duplication in Char by more than 130 lines
 and improves the performance of some methods a little bit.

 Somebody please have a short look...

 Happy Hacking
 Andreas

 ___
 Mono-devel-list mailing list
 Mono-devel-list@lists.ximian.com
 http://lists.ximian.com/mailman/listinfo/mono-devel-list


___
Mono-devel-list mailing list
Mono-devel-list@lists.ximian.com
http://lists.ximian.com/mailman/listinfo/mono-devel-list

Re: [Mono-dev] [Patch] to significantly shorten Char.cs

2008-06-26 Thread Andreas Nahr 
Well the main improvement is the shortening of the String-Parameter methods.
It does not affect performance negatively as far as I measured because the
methods now get inlined but were too large before.
I know that the code wasn't duplicated for performance reasons before,
because I actually wrote that code. Well most of Char anyways... ;)

IsWhiteSpace is now somewhat faster. Similar Prog from below (do not have
the original anymore) went from 21 sec total to 16,5 sec (AMD 3,2Ghz XP)
(testing one positive + one negative case). It simply saves some opcodes.

public static void Main()
{
int tickCount = Environment.TickCount;
for (int i = 0; i  0x3b9aca00; i++)
{
char.IsWhiteSpace('v');
}
Console.WriteLine((int) (Environment.TickCount - tickCount));
tickCount = Environment.TickCount;
for (int j = 0; j  0x3b9aca00; j++)
{
char.IsWhiteSpace(' ');
}
Console.WriteLine((int) (Environment.TickCount - tickCount));
}

And no, IsWhiteSpace did not get optimized away ;)

 -Ursprüngliche Nachricht-
 Von: [EMAIL PROTECTED] [mailto:mono-devel-list-
 [EMAIL PROTECTED] Im Auftrag von Atsushi Eno
 Gesendet: Donnerstag, 26. Juni 2008 23:25
 An: 'mono-devel'
 Betreff: Re: [Mono-dev] [Patch] to significantly shorten Char.cs
 
 I have some comments even without reading your application/octet-stream
 patch (I ususally don't read them).
 
 - Sometimes we have some code pieces that look like duplicates
exactly because of performance improvements.
 - For performance improvements there should be some benchmark
results with sources.
 
 Atsushi Eno
 
 
 Andreas Nahr wrote:
  The attached patch reduces code duplication in Char by more than 130
 lines
  and improves the performance of some methods a little bit.
 
  Somebody please have a short look...
 
  Happy Hacking
  Andreas
 
 
  -
 ---
 
  ___
  Mono-devel-list mailing list
  Mono-devel-list@lists.ximian.com
  http://lists.ximian.com/mailman/listinfo/mono-devel-list
 
 ___
 Mono-devel-list mailing list
 Mono-devel-list@lists.ximian.com
 http://lists.ximian.com/mailman/listinfo/mono-devel-list

___
Mono-devel-list mailing list
Mono-devel-list@lists.ximian.com
http://lists.ximian.com/mailman/listinfo/mono-devel-list