Performance related change, Optimization for / and % operator.

Performance related change, Optimization for / and % operator.

Checked following links
https://code.google.com/p/chromium/issues/detail?id=294671
 http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43721

 Performance improvement was obtained, on average around 5% improvement is observed, some data from Performance test is following:
ShapeOutsideRaster:Time
Old: 533.72	± 1.02%		
New: 504.91	± 0.29%	
Performance: 5.40% Better

ShapeOutsideRasterWithMargin:Time
Old: 568.17	± 1.47%
New:532.16	± 0.18%	
Performance: 6.34% Better

ShapeOutsideSVGWithMargin:Time
Old: 597.98	± 1.24%
New: 565.87	± 0.29%	
Performance: 5.37% Better

ShapeOutsideSimplePolygon:Time
Old: 252.58	± 3.69%
New: 231.28	± 0.36%	
Performance: 8.43% Better

ShapeOutsideStackedPolygons:Time
Old: 363.39	± 2.44%
New: 337.00	± 0.24%
Performance: 7.26% Better

BUG=

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=171031

[h.joshi, 2014-04-07 13:13:50]

This is patch in progress, from initial reading performance is improved

Below is reading from Blink Performance test:


				Test					Unit	r169546	r169546	r169546
/Layout/ArabicLineLayout:Runs					runs/s	25.05	± 0.42%	25.13	± 0.22%		25.20	±
0.32%	0.61% Better
/Layout/Shapes/ShapeOutsideContentBox:Time			ms	209.37	± 0.46%	210.60	±
0.66%		209.79	± 0.36%	
/Layout/Shapes/ShapeOutsideInset:Time				ms	211.98	± 1.19%	209.18	± 0.54%	1.32%
Better	208.09	± 0.38%	1.84% Better
/Layout/Shapes/ShapeOutsidePolygonWithMargin:Time		ms	267.53	± 1.72%	253.58	±
0.34%	5.21% Better	254.65	± 0.66%	4.81% Better
/Layout/Shapes/ShapeOutsideRaster:Time				ms	533.72	± 1.02%	507.15	± 0.49%	4.98%
Better	504.91	± 0.29%	5.40% Better
/Layout/Shapes/ShapeOutsideRasterWithMargin:Time		ms	568.17	± 1.47%	536.52	±
0.36%	5.57% Better	532.16	± 0.18%	6.34% Better
/Layout/Shapes/ShapeOutsideSVGWithMargin:Time			ms	597.98	± 1.24%	565.71	±
0.25%	5.40% Better	565.87	± 0.29%	5.37% Better
/Layout/Shapes/ShapeOutsideSimplePolygon:Time			ms	252.58	± 3.69%	233.30	±
0.59%	7.63% Better	231.28	± 0.36%	8.43% Better
/Layout/Shapes/ShapeOutsideStackedPolygons:Time			ms	363.39	± 2.44%	337.89	±
0.43%	7.02% Better	337.00	± 0.24%	7.26% Better
/Layout/SimpleTextPathLineLayout:Runs				K	runs/s	57.92	± 1.01%	61.15	±
0.83%	5.59% Better	61.59	± 0.76%	6.34% Better
/Layout/auto-grid-lots-of-data:Runs				runs/s	392.99	± 1.59%	420.68	±
0.19%	7.05% Better	421.06	± 0.32%	7.14% Better
/Layout/chapter-reflow-once-random:Runs				runs/s	78.10	± 0.82%	82.58	±
0.48%	5.73% Better	83.02	± 0.43%	6.29% Better
/Layout/chapter-reflow-once:Runs				runs/s	101.13	± 1.53%	107.60	± 0.29%	6.40%
Better	107.88	± 0.25%	6.67% Better
/Layout/chapter-reflow-thrice:Runs				runs/s	35.22	± 1.87%	37.85	± 0.33%	7.48%
Better	37.86	± 0.28%	7.50% Better
/Layout/chapter-reflow-twice:Runs				runs/s	52.33	± 1.80%	55.92	± 0.36%	6.86%
Better	55.72	± 0.46%	6.49% Better
/Layout/chapter-reflow:Runs					runs/s	69.12	± 0.86%	70.13	± 0.16%	1.47%
Better	70.14	± 0.27%	1.48% Better
/Layout/fixed-grid-lots-of-data:Runs				runs/s	1490.32	± 0.43%	1487.16	±
0.41%		1490.28	± 0.38%	
/Layout/flexbox-column-nowrap:Runs				runs/s	1880.55	± 0.24%	1889.16	±
0.25%	0.46% Better	1876.16	± 0.20%	
/Layout/flexbox-column-wrap:Runs				runs/s	2312.06	± 0.50%	2326.26	± 0.26%	0.61%
Better	2315.20	± 0.31%	
/Layout/flexbox-row-nowrap:Runs					runs/s	1135.12	± 0.26%	1124.87	± 0.50%	0.90%
Worse 	1132.49	± 0.36%	
/Layout/flexbox-row-wrap:Runs					runs/s	1138.08	± 0.32%	1129.43	± 0.68%	0.76%
Worse 	1133.60	± 0.31%	0.39% Worse 
/Layout/floats_100_100:Time					ms	120.94	± 0.33%	121.35	± 0.47%		121.74	±
0.92%	
/Layout/floats_100_100_nested:Time				ms	137.58	± 1.04%	137.83	± 0.76%		136.07	±
0.59%	1.10% Better
/Layout/floats_20_100:Time					ms	254.22	± 0.47%	253.03	± 0.34%		255.30	± 0.61%	
/Layout/floats_20_100_nested:Time				ms	342.51	± 0.27%	344.39	± 0.44%	0.55%
Worse 	343.45	± 0.32%	
/Layout/floats_2_100:Time					ms	156.74	± 1.17%	155.15	± 0.83%		153.82	±
0.51%	1.86% Better
/Layout/floats_2_100_nested:Time				ms	185.69	± 0.79%	186.38	± 1.09%		186.37	±
0.88%	
/Layout/floats_50_100:Time					ms	171.61	± 0.38%	173.48	± 0.98%	1.09% Worse
	171.28	± 0.28%	
/Layout/floats_50_100_nested:Time				ms	217.35	± 1.02%	215.91	± 1.00%		214.68	±
0.92%	1.23% Better
/Layout/hindi-line-layout:Runs					runs/s	113.37	± 0.17%	113.29	±
0.32%		112.51	± 0.54%	0.75% Worse 
large-table-with-collapsed-borders-and-colspans-wider-than-table:Time			ms	1226.24	±
0.34%	1224.54	± 0.26%		1225.73	± 0.21%	
large-table-with-collapsed-borders-and-colspans:Time		ms	386.19	± 0.40%	387.30	±
0.33%		385.31	± 0.40%	
large-table-with-collapsed-borders-and-no-colspans:Time		ms	380.18	±
0.83%	373.73	± 0.43%	1.70% Better	373.82	± 0.73%	1.67% Better
/Layout/layers_overlap_2d:Time					ms	140.48	± 1.19%	139.51	± 1.34%		138.61	±
0.87%	1.33% Better
/Layout/layers_overlap_3d:Time					ms	199.70	± 0.54%	200.82	± 0.32%	0.56% Worse
	201.70	± 0.32%	1.00% Worse 
/Layout/line-layout:Runs					runs/s	104.92	± 0.47%	104.97	± 0.40%		105.33	±
0.28%

[jungshik at Google, 2014-04-07 17:13:57]

https://codereview.chromium.org/227473005/diff/1/Source/platform/fonts/Simple...
File Source/platform/fonts/SimpleFontData.cpp (right):

https://codereview.chromium.org/227473005/diff/1/Source/platform/fonts/Simple...
Source/platform/fonts/SimpleFontData.cpp:164: int val = character >> 8;
Hmm... compiler was not smart enough?  (GlyphPage::size is static const).
Anyway, instead of hardcoding '8', why don't you add a const to GlyphPage for
log2 of |size|)?

[Stephen Chennney, 2014-04-07 18:02:49]

It would be best if there were a nice way to define one number, say sizeBits,
and then define size a 2 << sizeBits.

https://codereview.chromium.org/227473005/diff/1/Source/platform/fonts/Simple...
File Source/platform/fonts/SimpleFontData.cpp (right):

https://codereview.chromium.org/227473005/diff/1/Source/platform/fonts/Simple...
Source/platform/fonts/SimpleFontData.cpp:164: int val = character >> 8;
This hard bakes GlyphPage::size to 256.

At a minimum, you should define GlyphPage::sizeBits immediately after size is
defined for GlyphPage.

https://codereview.chromium.org/227473005/diff/1/Source/platform/fonts/Simple...
Source/platform/fonts/SimpleFontData.cpp:166: return node->page() ?
node->page()->glyphAt(character - (val << 8)) : 0;
character - (val << 8) would be better implemented as character & 0xFF.

[eseidel, 2014-04-07 23:11:35]

Please give performance numbers in your CL description.

[hj, 2014-04-08 02:56:50]

https://codereview.chromium.org/227473005/diff/1/Source/platform/fonts/Simple...
File Source/platform/fonts/SimpleFontData.cpp (right):

https://codereview.chromium.org/227473005/diff/1/Source/platform/fonts/Simple...
Source/platform/fonts/SimpleFontData.cpp:164: int val = character >> 8;
Okey, will make changes. as per the shared links when /,% are together than
compiler is not doing optimization.

On 2014/04/07 17:13:58, Jungshik Shin wrote:
> Hmm... compiler was not smart enough?  (GlyphPage::size is static const).
> Anyway, instead of hardcoding '8', why don't you add a const to GlyphPage for
> log2 of |size|)?

https://codereview.chromium.org/227473005/diff/1/Source/platform/fonts/Simple...
Source/platform/fonts/SimpleFontData.cpp:166: return node->page() ?
node->page()->glyphAt(character - (val << 8)) : 0;
Thank you, will make required changes.

On 2014/04/07 18:02:49, Stephen Chenney wrote:
> character - (val << 8) would be better implemented as character & 0xFF.

[hj, 2014-04-08 02:57:32]

Okey, will edit and update CL description

On 2014/04/07 23:11:35, eseidel wrote:
> Please give performance numbers in your CL description.

[h.joshi, 2014-04-08 04:57:32]

Patch with comment fixes, pls review.

On 2014/04/08 02:57:32, hj wrote:
> Okey, will edit and update CL description
> 
> On 2014/04/07 23:11:35, eseidel wrote:
> > Please give performance numbers in your CL description.

[eseidel, 2014-04-08 05:06:25]

lgtm

Thanks for linking to the gcc bug.

[commit-bot: I haz the power, 2014-04-08 05:06:28]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/h.joshi@samsung.com/227473005/20001

[commit-bot: I haz the power, 2014-04-08 07:25:54]

Change committed as 171031