Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(3262)

Issue 11905: threadedceval5.patch (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
15 years, 2 months ago by Jeffrey Yasskin
Modified:
14 years, 8 months ago
CC:
solipsis_pitrou.net, mal_egenix.com
Base URL:
http://svn.python.org/view/*checkout*/python/branches/py3k/
Visibility:
Public.

Description

Review of Antoine's computed-goto opcode dispatch patch: http://bugs.python.org/issue4753

Patch Set 1 #

Total comments: 19

Patch Set 2 : threadedceval6.patch #

Unified diffs Side-by-side diffs Delta from patch set Stats (+690 lines, -198 lines) Patch
M Makefile.pre.in View 1 2 chunks +15 lines, -0 lines 0 comments Download
M Python/ceval.c View 1 50 chunks +308 lines, -188 lines 0 comments Download
A Python/makeopcodetargets.py View 1 chunk +44 lines, -0 lines 0 comments Download
A Python/opcode_targets.h View 1 chunk +258 lines, -0 lines 0 comments Download
M configure.in View 1 chunk +16 lines, -0 lines 0 comments Download
M pyconfig.h.in View 4 chunks +49 lines, -10 lines 0 comments Download

Messages

Total messages: 4
Jeffrey Yasskin
Overall, this looks very good. Thanks for doing it! http://codereview.appspot.com/11905/diff/1/2 File Python/ceval.c (left): http://codereview.appspot.com/11905/diff/1/2#oldcode669 Line ...
15 years, 2 months ago (2009-01-12 20:05:54 UTC) #1
Antoine Pitrou
Thanks for your comments! http://codereview.appspot.com/11905/diff/1/2 File Python/ceval.c (left): http://codereview.appspot.com/11905/diff/1/2#oldcode669 Line 669: #define PREDICTED(op) PRED_##op: next_instr++ ...
15 years, 2 months ago (2009-01-12 21:19:48 UTC) #2
Jeffrey Yasskin
Hi Marc, there's a question for you below. Antoine, I look forward to an updated ...
15 years, 2 months ago (2009-01-14 16:56:35 UTC) #3
Antoine Pitrou
15 years, 2 months ago (2009-01-14 18:15:08 UTC) #4
On 2009/01/14 16:56:35, jyasskin wrote:
> That said, having an explicit option with an auto-detected
> default seems like a good idea, and we could initially default it to 'off' in
> order to shake out any bugs.

Hmm, my configure skills are exactly equal to 0.0, so the simpler the better :)
I'll take a look but I might need help.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b