Optimize set_syscall_retval for riscv
ClosedPublic
Actions

Authored by trasz on Oct 28 2020, 7:43 PM.

Details

Reviewers

mhorne
kp

Group Reviewers

riscv

Commits

rS367138: Optimize set_syscall_retval for riscv by predicting the return

Summary

Optimize set_syscall_retval for riscv by predicting the return
value to be zero.

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 34448
Build 31550: arc lint + arc unit

Event Timeline

trasz created this revision.Oct 28 2020, 7:43 PM

Herald added a subscriber: imp. · View Herald TranscriptOct 28 2020, 7:43 PM

Harbormaster completed remote builds in B34448: Diff 78864.Oct 28 2020, 7:43 PM

trasz requested review of this revision.Oct 28 2020, 7:43 PM

trasz added a reviewer: riscv.Oct 28 2020, 7:44 PM

LGTM

This revision is now accepted and ready to land.Oct 28 2020, 8:48 PM

Do we see measurable performance improvements with this?

(Looking at the generated code it does seem to be better, but measurements would make me feel better.)

I wouldn’t expect any visible results. The syscall path overhead is still huge, and there’s not a single reason for it - it’s a “death by thousand cuts” situation, which needs to be resolved in small chunks, like this one.

Closed by commit rS367138: Optimize set_syscall_retval for riscv by predicting the return (authored by trasz). · Explain WhyOct 29 2020, 3:36 PM

This revision was automatically updated to reflect the committed changes.

trasz added a commit: rS367138: Optimize set_syscall_retval for riscv by predicting the return.

Revision Contents
Changeset List

Path

Size

sys/

riscv/

vm_machdep.c

8 lines

Diff 78864

View Options

sys/riscv/riscv/vm_machdep.c

import types		import types
from typing import Any		from typing import Any
from typing import Dict		from typing import Dict
from typing import List		from typing import List
from typing import NamedTuple		from typing import NamedTuple
from typing import Optional		from typing import Optional
from typing import Tuple		from typing import Tuple

		from atf_python.ktest import generate_ktests
		ngieUnsubmitted Not Done Inline Actions Relative imports are better, assuming this is a (namespace )? package. It makes it easier to lint items like this without installing everything beforehand. ngie: Relative imports are better, assuming this is a (namespace )? package. It makes it easier to…
		melifaroAuthorUnsubmitted Done Inline Actions I don't have a hard opinion on this. Currently, it's a base-only "package" being installed by the FreeBSD makefile. In its current form, the directory structure there somewhat replicates the base system layout, resulting in directory depth being 3-4 items. The imports from the depth-most files may look a bit ugly. Also, the other parts of the module use absolute imports. I'd suggest discussing it as a whole, to make the module look consistent. If you feel that the relative imports are the way to go, I'm happy to review/accept such a diff. melifaro: I don't have a hard opinion on this. Currently, it's a base-only "package" being installed by…
from atf_python.utils import nodeid_to_method_name		from atf_python.utils import nodeid_to_method_name

import pytest		import pytest
import os		import os


class ATFCleanupItem(pytest.Item):		class ATFCleanupItem(pytest.Item):
def runtest(self):		def runtest(self):
Show All 20 Lines	def __init__(self, obj, has_cleanup):
# Use nodeid without name to properly name class-derived tests		# Use nodeid without name to properly name class-derived tests
self.ident = obj.nodeid.split("::", 1)[1]		self.ident = obj.nodeid.split("::", 1)[1]
self.description = self._get_test_description(obj)		self.description = self._get_test_description(obj)
self.has_cleanup = has_cleanup		self.has_cleanup = has_cleanup
self.obj = obj		self.obj = obj

def _get_test_description(self, obj):		def _get_test_description(self, obj):
"""Returns first non-empty line from func docstring or func name"""		"""Returns first non-empty line from func docstring or func name"""
		if getattr(obj, "descr", None) is not None:
		return getattr(obj, "descr")
docstr = obj.function.__doc__		docstr = obj.function.__doc__
if docstr:		if docstr:
for line in docstr.split("\n"):		for line in docstr.split("\n"):
if line:		if line:
return line		return line
return obj.name		return obj.name

@staticmethod		@staticmethod
Show All 13 Lines	def _convert_user_mark(mark, obj, ret: Dict):
# Yes, so we ask kyua to run us under root instead		# Yes, so we ask kyua to run us under root instead
# It is up to the implementation to switch back to the desired		# It is up to the implementation to switch back to the desired
# user		# user
ret["require.user"] = "root"		ret["require.user"] = "root"
else:		else:
ret["require.user"] = username		ret["require.user"] = username

def _convert_marks(self, obj) -> Dict[str, Any]:		def _convert_marks(self, obj) -> Dict[str, Any]:
wj_func = lambda x: " ".join(x) # noqa: E731		wj_func = lambda x: " ".join(x) # noqa: E731
		Lint: PEP8 E731 Inline Actions do not assign a lambda expression, use a def Lint: PEP8 E731: do not assign a lambda expression, use a def
_map: Dict[str, Dict] = {		_map: Dict[str, Dict] = {
		Lint: PEP8 E701 Inline Actions multiple statements on one line (colon) Lint: PEP8 E701: multiple statements on one line (colon)
"require_user": {"handler": self._convert_user_mark},		"require_user": {"handler": self._convert_user_mark},
"require_arch": {"name": "require.arch", "fmt": wj_func},		"require_arch": {"name": "require.arch", "fmt": wj_func},
"require_diskspace": {"name": "require.diskspace"},		"require_diskspace": {"name": "require.diskspace"},
"require_files": {"name": "require.files", "fmt": wj_func},		"require_files": {"name": "require.files", "fmt": wj_func},
"require_machine": {"name": "require.machine", "fmt": wj_func},		"require_machine": {"name": "require.machine", "fmt": wj_func},
"require_memory": {"name": "require.memory"},		"require_memory": {"name": "require.memory"},
"require_progs": {"name": "require.progs", "fmt": wj_func},		"require_progs": {"name": "require.progs", "fmt": wj_func},
"timeout": {},		"timeout": {},
Show All 21 Lines	def as_lines(self) -> List[str]:
ret.append("has.cleanup: true")		ret.append("has.cleanup: true")
for key, value in self._convert_marks(self.obj).items():		for key, value in self._convert_marks(self.obj).items():
ret.append("{}: {}".format(key, value))		ret.append("{}: {}".format(key, value))
return ret		return ret


class ATFHandler(object):		class ATFHandler(object):
class ReportState(NamedTuple):		class ReportState(NamedTuple):
state: str		state: str
		Lint: PEP8 E701 Inline Actions multiple statements on one line (colon) Lint: PEP8 E701: multiple statements on one line (colon)
reason: str		reason: str
		Lint: PEP8 E701 Inline Actions multiple statements on one line (colon) Lint: PEP8 E701: multiple statements on one line (colon)

def __init__(self, report_file_name: Optional[str]):		def __init__(self, report_file_name: Optional[str]):
self._tests_state_map: Dict[str, ReportStatus] = {}		self._tests_state_map: Dict[str, ReportStatus] = {}
		Lint: PEP8 E701 Inline Actions multiple statements on one line (colon) Lint: PEP8 E701: multiple statements on one line (colon)
self._report_file_name = report_file_name		self._report_file_name = report_file_name
self._report_file_handle = None		self._report_file_handle = None

def setup_configure(self):		def setup_configure(self):
fname = self._report_file_name		fname = self._report_file_name
if fname:		if fname:
self._report_file_handle = open(fname, mode="w")		self._report_file_handle = open(fname, mode="w")

Show All 31 Lines	class ATFHandler(object):
def _generate_test_cleanups(self, items):		def _generate_test_cleanups(self, items):
new_items = []		new_items = []
for obj in items:		for obj in items:
if self.has_object_cleanup(obj):		if self.has_object_cleanup(obj):
self.override_runtest(obj)		self.override_runtest(obj)
new_items.append(obj)		new_items.append(obj)
items.clear()		items.clear()
items.extend(new_items)		items.extend(new_items)

		def expand_tests(self, collector, name, obj):
		return generate_ktests(collector, name, obj)

def modify_tests(self, items, config):		def modify_tests(self, items, config):
if config.option.atf_cleanup:		if config.option.atf_cleanup:
self._generate_test_cleanups(items)		self._generate_test_cleanups(items)

def list_tests(self, tests: List[str]):		def list_tests(self, tests: List[str]):
print('Content-Type: application/X-atf-tp; version="1"')		print('Content-Type: application/X-atf-tp; version="1"')
print()		print()
▲ Show 20 Lines • Show All 111 Lines • ▼ Show 20 Lines	def write_report(self):
else:		else:
line = "{}: {}".format(test.state, test.reason)		line = "{}: {}".format(test.state, test.reason)
print(line, file=self._report_file_handle)		print(line, file=self._report_file_handle)
self._report_file_handle.close()		self._report_file_handle.close()

@staticmethod		@staticmethod
def get_atf_vars() -> Dict[str, str]:		def get_atf_vars() -> Dict[str, str]:
px = "_ATF_VAR_"		px = "_ATF_VAR_"
return {k[len(px):]: v for k, v in os.environ.items() if k.startswith(px)}		return {k[len(px):]: v for k, v in os.environ.items() if k.startswith(px)}
		Lint: PEP8 E501 Inline Actions line too long (82 > 79 characters) Lint: PEP8 E501: line too long (82 > 79 characters)

Optimize set_syscall_retval for riscvClosedPublicActions