single shared ticker #37109

smontanaro · 2002-08-30T02:01:08Z

BPO	602191
Nosy	@gvanrossum, @tim-one, @smontanaro
Files	ticker.patch

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = 'https://github.com/smontanaro'
closed_at = <Date 2002-09-03.20:25:06.000>
created_at = <Date 2002-08-30.02:01:08.000>
labels = ['interpreter-core']
title = 'single shared ticker'
updated_at = <Date 2002-09-03.20:25:06.000>
user = 'https://github.com/smontanaro'

bugs.python.org fields:

activity = <Date 2002-09-03.20:25:06.000>
actor = 'skip.montanaro'
assignee = 'skip.montanaro'
closed = True
closed_date = None
closer = None
components = ['Interpreter Core']
creation = <Date 2002-08-30.02:01:08.000>
creator = 'skip.montanaro'
dependencies = []
files = ['4549']
hgrepos = []
issue_num = 602191
keywords = ['patch']
message_count = 15.0
messages = ['41069', '41070', '41071', '41072', '41073', '41074', '41075', '41076', '41077', '41078', '41079', '41080', '41081', '41082', '41083']
nosy_count = 3.0
nosy_names = ['gvanrossum', 'tim.peters', 'skip.montanaro']
pr_nums = []
priority = 'normal'
resolution = 'accepted'
stage = None
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue602191'
versions = []

smontanaro · 2002-08-30T02:01:08Z

Per discussion on python-dev, here's a patch that gets rid of the
per-thread ticker, instead sharing a single one amongst all threads
(and long ints).

smontanaro · 2002-08-30T04:15:40Z

Logged In: YES
user_id=44345

Here's an updated patch. It creates two internal globals, _Py_Ticker and
_Py_CheckInterval. They are accessed from sysmodule.c, ceval.c and
longobject.c

smontanaro · 2002-08-30T15:43:54Z

Logged In: YES
user_id=44345

minor correction to the patch - initialize both _Py_CheckInterval and
_Py_Ticker to 10 so direct pystones comparisons can be made.

tim-one · 2002-08-30T16:01:18Z

Logged In: YES
user_id=31435

I'd be more interested in a patch that also implemented
Jeremy's suggestion.

smontanaro · 2002-08-30T16:53:36Z

Logged In: YES
user_id=44345

Here's a new version that includes Jeremy's shortcut. With
_Py_CheckInterval initialized to 10 here are the pystones numbers I get:

with my initial patch & Jeremy's ticker shortcut:

Pystone(1.1) time for 50000 passes = 7.67
This machine benchmarks at 6518.9 pystones/second
Pystone(1.1) time for 50000 passes = 7.67
This machine benchmarks at 6518.9 pystones/second
Pystone(1.1) time for 50000 passes = 7.65
This machine benchmarks at 6535.95 pystones/second

back to just my initial patch without the shortcut:

Pystone(1.1) time for 50000 passes = 7.59
This machine benchmarks at 6587.62 pystones/second
Pystone(1.1) time for 50000 passes = 7.56
This machine benchmarks at 6613.76 pystones/second
Pystone(1.1) time for 50000 passes = 7.56
This machine benchmarks at 6613.76 pystones/second

I'm perplexed by the performance difference. Again, I think these
performance numbers should be checked by some other people. BTW, I
configured with

OPT=-O3 ../configure

in my build directory. I'm using gcc 2.96 and glibc 2.2.4.

tim-one · 2002-08-30T17:10:27Z

Logged In: YES
user_id=31435

Note that the ticker has to be declared volatile. Also

"""
Py_MakePendingCalls should also be changed then to
reset ticker to 0 in its "early exit because the coincidences
I'm relying on haven't happened yet" cases.
"""

You can't predict whether ceval will slow down or speed up,
so don't bother with being confused <wink>. Get the code
right first. Good Things will follow.

smontanaro · 2002-08-30T18:26:23Z

Logged In: YES
user_id=44345

bump initial check interval to 100, per request from Tim & Guido.

smontanaro · 2002-08-30T19:08:37Z

Logged In: YES
user_id=44345

This version declares the ticker volatile. Obvious performance hit. Does it
need to be volatile if the Jeremy's shortcut is removed?

tim-one · 2002-08-30T20:02:00Z

Logged In: YES
user_id=31435

A performance hit compared to what? There are half a
dozen variations stacked up now. _Py_CheckInterval is
back to 10 in the latest patch, and perhaps that has
something to do with it.

_Py_CheckInterval needn't be delcared volatile, BTW; i.e.,
take the "volatile" out of

+ PyAPI_DATA(volatile int) _Py_CheckInterval;

I can't time this today, but you should be just as keen to
get x-platform verification when claiming a performance hit
as when claiming a performance boost. Chances are that it
will slobber all over the place across compilers; ceval is
extremely touchy. I'm sitting on a major slowdown under
MSCV6 after the SET_LINENO thing, and I'm not panicked
about that <wink>.

smontanaro · 2002-08-30T20:58:39Z

Logged In: YES
user_id=44345

Sorry, compared to the previous version (_Py_CheckInterval == 10,
Jeremy's shortcut installed, but _Py_Ticker not yet declared volatile). My
first submission didn't include any performance tests. That was the first
thing Guido asked for, so I started running pystones with each change.

Let me know what, if anything, you'd like me to report performance-wise.

Skip

smontanaro · 2002-08-30T22:36:33Z

Logged In: YES
user_id=44345

I just went back and re-ran everything. Here are the results.

methodology:

adjust source
recompile (always gcc 2.96 w/ -O3)
run pystones twice, ignoring values
run pystones three times, reporting values

------------------------------------------------------------------------------
baseline (none of this stuff)

Pystone(1.1) time for 50000 passes = 7.68
This machine benchmarks at 6510.42 pystones/second
Pystone(1.1) time for 50000 passes = 7.68
This machine benchmarks at 6510.42 pystones/second
Pystone(1.1) time for 50000 passes = 7.67
This machine benchmarks at 6518.9 pystones/second

------------------------------------------------------------------------------
baseline + check interval set to 100

Pystone(1.1) time for 50000 passes = 7.64
This machine benchmarks at 6544.5 pystones/second
Pystone(1.1) time for 50000 passes = 7.63
This machine benchmarks at 6553.08 pystones/second
Pystone(1.1) time for 50000 passes = 7.62
This machine benchmarks at 6561.68 pystones/second

------------------------------------------------------------------------------
global, volatile _Py_Ticker == 10
global, nonvolatile _Py_CheckInterval == 10
Jeremy's Py_AddPendingCall shortcut enabled

Pystone(1.1) time for 50000 passes = 7.67
This machine benchmarks at 6518.9 pystones/second
Pystone(1.1) time for 50000 passes = 7.7
This machine benchmarks at 6493.51 pystones/second
Pystone(1.1) time for 50000 passes = 7.68
This machine benchmarks at 6510.42 pystones/second

------------------------------------------------------------------------------
global, volatile _Py_Ticker == 100
global, nonvolatile _Py_CheckInterval == 100
Jeremy's Py_AddPendingCall shortcut enabled

Pystone(1.1) time for 50000 passes = 7.67
This machine benchmarks at 6518.9 pystones/second
Pystone(1.1) time for 50000 passes = 7.6
This machine benchmarks at 6578.95 pystones/second
Pystone(1.1) time for 50000 passes = 7.62
This machine benchmarks at 6561.68 pystones/second

------------------------------------------------------------------------------
global, nonvolatile _Py_Ticker == 100
global, nonvolatile _Py_CheckInterval == 100
Jeremy's Py_AddPendingCall shortcut disabled

Pystone(1.1) time for 50000 passes = 7.67
This machine benchmarks at 6518.9 pystones/second
Pystone(1.1) time for 50000 passes = 7.66
This machine benchmarks at 6527.42 pystones/second
Pystone(1.1) time for 50000 passes = 7.64
This machine benchmarks at 6544.5 pystones/second

gvanrossum · 2002-09-03T18:21:38Z

Logged In: YES
user_id=6380

FWIW, I measure a similar slowdown with the latest patch,
compared to current CVS.

But I still think it's good to check this in now, Skip. Then
in a separate patch we'll increment Py_CheckInterval to 100.

tim-one · 2002-09-03T18:46:54Z

Logged In: YES
user_id=31435

I get a 1.5% speedup on Win2K, baseline versus whatever
this patch does. Go for it,

Skip, as I said before,

"""
_Py_CheckInterval needn't be delcared volatile, BTW; i.e.,
take the "volatile" out of

+ PyAPI_DATA(volatile int) _Py_CheckInterval;
"""

The patch won't compile under MSVC6 unless this is done
(as is, the declaration conflicts with the definition).

smontanaro · 2002-09-03T20:13:05Z

Logged In: YES
user_id=44345

Implemented as
Include/pystate.h 2.20
Include/ceval.h 2.46
Python/ceval.c 2.334
Python/sysmodule.c 2.110
Objects/longobject.c 1.143

smontanaro · 2002-09-03T20:25:06Z

Logged In: YES
user_id=44345

oops, and Python/pystate.c 2.21

smontanaro closed this as completed Aug 30, 2002

smontanaro self-assigned this Aug 30, 2002

smontanaro added the interpreter-core (Objects, Python, Grammar, and Parser dirs) label Aug 30, 2002

smontanaro closed this as completed Aug 30, 2002

smontanaro self-assigned this Aug 30, 2002

smontanaro added the interpreter-core (Objects, Python, Grammar, and Parser dirs) label Aug 30, 2002

ezio-melotti transferred this issue from another repository Apr 9, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

single shared ticker #37109

single shared ticker #37109

smontanaro commented Aug 30, 2002

smontanaro commented Aug 30, 2002

smontanaro commented Aug 30, 2002

smontanaro commented Aug 30, 2002

tim-one commented Aug 30, 2002

smontanaro commented Aug 30, 2002

tim-one commented Aug 30, 2002

smontanaro commented Aug 30, 2002

smontanaro commented Aug 30, 2002

tim-one commented Aug 30, 2002

smontanaro commented Aug 30, 2002

smontanaro commented Aug 30, 2002

gvanrossum commented Sep 3, 2002

tim-one commented Sep 3, 2002

smontanaro commented Sep 3, 2002

smontanaro commented Sep 3, 2002

single shared ticker #37109

single shared ticker #37109

Comments

smontanaro commented Aug 30, 2002

smontanaro commented Aug 30, 2002

smontanaro commented Aug 30, 2002

smontanaro commented Aug 30, 2002

tim-one commented Aug 30, 2002

smontanaro commented Aug 30, 2002

tim-one commented Aug 30, 2002

smontanaro commented Aug 30, 2002

smontanaro commented Aug 30, 2002

tim-one commented Aug 30, 2002

smontanaro commented Aug 30, 2002

smontanaro commented Aug 30, 2002

gvanrossum commented Sep 3, 2002

tim-one commented Sep 3, 2002

smontanaro commented Sep 3, 2002

smontanaro commented Sep 3, 2002