Skip to content

GH-103082: Code cleanup from review. #103474

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 29, 2023
Merged

GH-103082: Code cleanup from review. #103474

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fb56027
Code cleanup
markshannon Apr 12, 2023
63f7319
Follow PEP 7
markshannon Apr 12, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions Include/internal/pycore_frame.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,9 +139,9 @@ _PyFrame_GetLocalsArray(_PyInterpreterFrame *frame)
}

/* Fetches the stack pointer, and sets stacktop to -1.
Having stacktop <= 0 ensures that invalid
values are not visible to the cycle GC.
We choose -1 rather than 0 to assist debugging. */
Having stacktop <= 0 ensures that invalid
values are not visible to the cycle GC.
We choose -1 rather than 0 to assist debugging. */
static inline PyObject**
_PyFrame_GetStackPointer(_PyInterpreterFrame *frame)
{
Expand Down
65 changes: 34 additions & 31 deletions Python/instrumentation.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,13 +111,17 @@ static const uint8_t INSTRUMENTED_OPCODES[256] = {
};

static inline bool
opcode_has_event(int opcode) {
return opcode < INSTRUMENTED_LINE &&
INSTRUMENTED_OPCODES[opcode] > 0;
opcode_has_event(int opcode)
{
return (
opcode < INSTRUMENTED_LINE &&
INSTRUMENTED_OPCODES[opcode] > 0
);
}

static inline bool
is_instrumented(int opcode) {
is_instrumented(int opcode)
{
assert(opcode != 0);
assert(opcode != RESERVED);
return opcode >= MIN_INSTRUMENTED_OPCODE;
Expand Down Expand Up @@ -333,7 +337,8 @@ dump_monitors(const char *prefix, _Py_Monitors monitors, FILE*out)
/* Like _Py_GetBaseOpcode but without asserts.
* Does its best to give the right answer, but won't abort
* if something is wrong */
int get_base_opcode_best_attempt(PyCodeObject *code, int offset)
static int
get_base_opcode_best_attempt(PyCodeObject *code, int offset)
{
int opcode = _Py_OPCODE(_PyCode_CODE(code)[offset]);
if (INSTRUMENTED_OPCODES[opcode] != opcode) {
Expand Down Expand Up @@ -412,13 +417,15 @@ dump_instrumentation_data(PyCodeObject *code, int star, FILE*out)
assert(test); \
} while (0)

bool valid_opcode(int opcode) {
static bool
valid_opcode(int opcode)
{
if (opcode > 0 &&
opcode != RESERVED &&
opcode < 255 &&
_PyOpcode_OpName[opcode] &&
_PyOpcode_OpName[opcode][0] != '<'
) {
_PyOpcode_OpName[opcode][0] != '<')
{
return true;
}
return false;
Expand Down Expand Up @@ -544,11 +551,11 @@ de_instrument(PyCodeObject *code, int i, int event)
opcode_ptr = &code->_co_monitoring->lines[i].original_opcode;
opcode = *opcode_ptr;
}
if (opcode == INSTRUMENTED_INSTRUCTION) {
if (opcode == INSTRUMENTED_INSTRUCTION) {
opcode_ptr = &code->_co_monitoring->per_instruction_opcodes[i];
opcode = *opcode_ptr;
}
int deinstrumented = DE_INSTRUMENT[opcode];
int deinstrumented = DE_INSTRUMENT[opcode];
if (deinstrumented == 0) {
return;
}
Expand Down Expand Up @@ -775,8 +782,7 @@ add_line_tools(PyCodeObject * code, int offset, int tools)
{
assert(tools_is_subset_for_event(code, PY_MONITORING_EVENT_LINE, tools));
assert(code->_co_monitoring);
if (code->_co_monitoring->line_tools
) {
if (code->_co_monitoring->line_tools) {
code->_co_monitoring->line_tools[offset] |= tools;
}
else {
Expand All @@ -792,8 +798,7 @@ add_per_instruction_tools(PyCodeObject * code, int offset, int tools)
{
assert(tools_is_subset_for_event(code, PY_MONITORING_EVENT_INSTRUCTION, tools));
assert(code->_co_monitoring);
if (code->_co_monitoring->per_instruction_tools
) {
if (code->_co_monitoring->per_instruction_tools) {
code->_co_monitoring->per_instruction_tools[offset] |= tools;
}
else {
Expand All @@ -808,11 +813,10 @@ static void
remove_per_instruction_tools(PyCodeObject * code, int offset, int tools)
{
assert(code->_co_monitoring);
if (code->_co_monitoring->per_instruction_tools)
{
if (code->_co_monitoring->per_instruction_tools) {
uint8_t *toolsptr = &code->_co_monitoring->per_instruction_tools[offset];
*toolsptr &= ~tools;
if (*toolsptr == 0 ) {
if (*toolsptr == 0) {
de_instrument_per_instruction(code, offset);
}
}
Expand All @@ -837,7 +841,7 @@ call_one_instrument(
assert(tstate->tracing == 0);
PyObject *instrument = interp->monitoring_callables[tool][event];
if (instrument == NULL) {
return 0;
return 0;
}
int old_what = tstate->what_event;
tstate->what_event = event;
Expand All @@ -859,16 +863,15 @@ static const int8_t MOST_SIGNIFICANT_BITS[16] = {
3, 3, 3, 3,
};

/* We could use _Py_bit_length here, but that is designed for larger (32/64) bit ints,
and can perform relatively poorly on platforms without the necessary intrinsics. */
/* We could use _Py_bit_length here, but that is designed for larger (32/64)
* bit ints, and can perform relatively poorly on platforms without the
* necessary intrinsics. */
static inline int most_significant_bit(uint8_t bits) {
assert(bits != 0);
if (bits > 15) {
return MOST_SIGNIFICANT_BITS[bits>>4]+4;
}
else {
return MOST_SIGNIFICANT_BITS[bits];
}
return MOST_SIGNIFICANT_BITS[bits];
}

static bool
Expand Down Expand Up @@ -996,8 +999,8 @@ _Py_call_instrumentation_2args(
int
_Py_call_instrumentation_jump(
PyThreadState *tstate, int event,
_PyInterpreterFrame *frame, _Py_CODEUNIT *instr, _Py_CODEUNIT *target
) {
_PyInterpreterFrame *frame, _Py_CODEUNIT *instr, _Py_CODEUNIT *target)
{
assert(event == PY_MONITORING_EVENT_JUMP ||
event == PY_MONITORING_EVENT_BRANCH);
assert(frame->prev_instr == instr);
Expand Down Expand Up @@ -1303,8 +1306,8 @@ initialize_line_tools(PyCodeObject *code, _Py_Monitors *all_events)
}
}

static
int allocate_instrumentation_data(PyCodeObject *code)
static int
allocate_instrumentation_data(PyCodeObject *code)
{

if (code->_co_monitoring == NULL) {
Expand Down Expand Up @@ -1398,7 +1401,7 @@ static const uint8_t super_instructions[256] = {

/* Should use instruction metadata for this */
static bool
is_super_instruction(int opcode) {
is_super_instruction(uint8_t opcode) {
return super_instructions[opcode] != 0;
}

Expand Down Expand Up @@ -1510,7 +1513,7 @@ _Py_Instrument(PyCodeObject *code, PyInterpreterState *interp)

#define C_RETURN_EVENTS \
((1 << PY_MONITORING_EVENT_C_RETURN) | \
(1 << PY_MONITORING_EVENT_C_RAISE))
(1 << PY_MONITORING_EVENT_C_RAISE))

#define C_CALL_EVENTS \
(C_RETURN_EVENTS | (1 << PY_MONITORING_EVENT_CALL))
Expand Down Expand Up @@ -1555,8 +1558,8 @@ static int
check_tool(PyInterpreterState *interp, int tool_id)
{
if (tool_id < PY_MONITORING_SYS_PROFILE_ID &&
interp->monitoring_tool_names[tool_id] == NULL
) {
interp->monitoring_tool_names[tool_id] == NULL)
{
PyErr_Format(PyExc_ValueError, "tool %d is not in use", tool_id);
return -1;
}
Expand Down
8 changes: 4 additions & 4 deletions Python/pystate.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -649,11 +649,11 @@ init_interpreter(PyInterpreterState *interp,
_PyGC_InitState(&interp->gc);
PyConfig_InitPythonConfig(&interp->config);
_PyType_InitCache(interp);
for(int i = 0; i < PY_MONITORING_UNGROUPED_EVENTS; i++) {
for (int i = 0; i < PY_MONITORING_UNGROUPED_EVENTS; i++) {
interp->monitors.tools[i] = 0;
}
for (int t = 0; t < PY_MONITORING_TOOL_IDS; t++) {
for(int e = 0; e < PY_MONITORING_EVENTS; e++) {
for (int e = 0; e < PY_MONITORING_EVENTS; e++) {
interp->monitoring_callables[t][e] = NULL;

}
Expand Down Expand Up @@ -797,11 +797,11 @@ interpreter_clear(PyInterpreterState *interp, PyThreadState *tstate)

Py_CLEAR(interp->audit_hooks);

for(int i = 0; i < PY_MONITORING_UNGROUPED_EVENTS; i++) {
for (int i = 0; i < PY_MONITORING_UNGROUPED_EVENTS; i++) {
interp->monitors.tools[i] = 0;
}
for (int t = 0; t < PY_MONITORING_TOOL_IDS; t++) {
for(int e = 0; e < PY_MONITORING_EVENTS; e++) {
for (int e = 0; e < PY_MONITORING_EVENTS; e++) {
Py_CLEAR(interp->monitoring_callables[t][e]);
}
}
Expand Down
2 changes: 1 addition & 1 deletion Python/specialize.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ print_spec_stats(FILE *out, OpcodeStats *stats)
PRIu64 "\n", i, j, val);
}
}
for(int j = 0; j < 256; j++) {
for (int j = 0; j < 256; j++) {
if (stats[i].pair_count[j]) {
fprintf(out, "opcode[%d].pair_count[%d] : %" PRIu64 "\n",
i, j, stats[i].pair_count[j]);
Expand Down