Issue 12340086: Force WriteByte to be inlined

Issue 12340086: Force WriteByte to be inlined (Closed)

Created:
7 years, 10 months ago by Cutch

Modified:
7 years, 9 months ago

Reviewers:
Søren Gjesse, srdjan, Ivan Posva

CC:
reviews_dartlang.org

Base URL:
https://dart.googlecode.com/svn/branches/bleeding_edge/dart

Visibility:
Public.

More Reviews

Description

Force WriteByte to be inlined Committed: https://code.google.com/p/dart/source/detail?r=19143

Patch Set 4 : Line re-appeared #

Total comments: 1

Created: 7 years, 10 months ago

Download [raw] [tar.bz2]

		Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+14 lines, -1 line)			Patch
	M	runtime/platform/globals.h	View	1 2 3	1 chunk	+13 lines, -0 lines	1 comment	Download
	M	runtime/vm/datastream.h	View	1	1 chunk	+1 line, -1 line	0 comments	Download

Messages

Total messages: 7 (0 generated)

Expand Messages | Collapse Messages

Cutch

This should fix: http://70.32.156.88:8080/Comparison#targetA%3Ddart%3BmachineTypeA%3Dlinux-x64%3BrevisionA%3D19001%3BpatchA%3DNone%3BtargetB%3Ddart%3BmachineTypeB%3Dlinux-x64%3BrevisionB%3D19000%3BpatchB%3DNone

7 years, 10 months ago (2013-02-26 21:58:24 UTC) #1

Ivan Posva

LGTM with comments. -Ivan https://codereview.chromium.org/12340086/diff/1/runtime/platform/globals.h File runtime/platform/globals.h (right): https://codereview.chromium.org/12340086/diff/1/runtime/platform/globals.h#newcode274 runtime/platform/globals.h:274: #ifdef _MSC_VER Please move this ...

7 years, 10 months ago (2013-02-26 22:23:49 UTC) #2

Ivan Posva

LGTM with comment. -Ivan https://codereview.chromium.org/12340086/diff/7001/runtime/platform/globals.h File runtime/platform/globals.h (left): https://codereview.chromium.org/12340086/diff/7001/runtime/platform/globals.h#oldcode162 runtime/platform/globals.h:162: Line disappeared.

7 years, 10 months ago (2013-02-26 22:59:15 UTC) #3

Cutch

$ ../tools/test.py -m all -a ia32,x64,simmips,simarm [06:30 | 100% | +26197 | - 0] I'll ...

7 years, 10 months ago (2013-02-27 00:00:58 UTC) #4

Søren Gjesse

lgtm https://codereview.chromium.org/12340086/diff/6004/runtime/platform/globals.h File runtime/platform/globals.h (right): https://codereview.chromium.org/12340086/diff/6004/runtime/platform/globals.h#newcode136 runtime/platform/globals.h:136: #else Wouldn't an empty define for DART_FORCE_INLINE work ...

7 years, 9 months ago (2013-02-27 12:57:44 UTC) #5

Cutch

Committed patchset #4 manually as r19143 (presubmit successful).

7 years, 9 months ago (2013-02-27 15:22:38 UTC) #6

Cutch

7 years, 9 months ago (2013-02-27 15:24:11 UTC) #7

Message was sent while issue was closed.

While we could define DART_FORCE_INLINE to be empty I think we would be
introducing a silent performance problem that would be hard to track down. We
don't support that many compilers, it should be easy.

Expand Messages | Collapse Messages