cfbolz changed the topic of #pypy to: PyPy, the flexible snake (IRC logs: https://botbot.me/freenode/pypy/ ) | use cffi for calling C | the secret reason for us trying to get PyPy users: to test the JIT well enough that we're somewhat confident about it
<mattip>
the JIT issue 2775 seems pretty serious. Any chance of getting some attention on it this week?
<mattip>
it may require more than one set of eyes, the code that is crashing is pretty old (pyjitpl.py, around the _nonstandard_virtualizable call)
<mattip>
we could for now just disable the sub_jitdriver.jit_merge_point in module/_sre/interp_sre.py, but that seems like a hack
<mattip>
it seems to be something about using an attribute in the greens, the only ones who do that are the various sre jit drivers, where they use "ctx.pattern"
amaury_ has quit [Ping timeout: 256 seconds]
<cfbolz>
mattip: I plan to work on it, yes
asmeurer__ has quit [Quit: asmeurer__]
<cfbolz>
mattip: it seems the interaction of two jitdrivers, one with greenfields, one with a virtualizable, was never completely thought through
<arigato>
I think we should compare the jit output with the two variants (done systematically inside all jitdrivers of rsre_core, too)
<cfbolz>
arigato: sorry, sorry
<arigato>
I suspect there is an extra getfield and guard_value to get the 'pattern', but I'm not sure these wouldn't be removed anyway
<arigato>
and anyway one getfield and one passing guard_value are probably an OK price to pay for removing this insane greenfields
<cfbolz>
Sounds like a plan. I can do that
<cfbolz>
I think Green Fields could also be implemented in a less weird way but I agree that I'm not sure it's worth it
<arigato>
I'm not quite sure that it works because pattern is declared _immutable_fields_=['pattern[*]']
<cfbolz>
Ah
<arigato>
maybe it needs to be sanitized too, as in set 'ctx.pattern' to a small object which itself has an immutable array of integers
<cfbolz>
That would be easy yes
<cfbolz>
If we want to fix the green fields we would have to make them stop using virtualizables infrastructure
<arigato>
ah, but wait
<arigato>
I fear that all 'ctx.pattern' after the jit_merge_point will not get a ConstPtr value
<cfbolz>
So we would have to hide the pattern into an accessor and put a promote there
<arigato>
or refactor the code so that we don't store 'pattern' on 'ctx' at all, just pass it around
<arigato>
but yes, the accessor with promote would work too
<cfbolz>
Right
<cfbolz>
But you would be in table of the moving greenfield.
<cfbolz>
?
<cfbolz>
Sorsy
<cfbolz>
You would be in favor of removing greenfields?
<arigato>
(there is mostly only the pat(index) method that reads self.pattern[index])
<arigato>
yes
<cfbolz>
OK. I can take care of all this
<arigato>
thanks!
<cfbolz>
:-)
<arigato>
I'd recommend to first kill a few 'ctx.pattern' greenfields in rsre_core and add a promote, and check that the JIT code still looks good
<arigato>
there is even rsre/test/test_jit.py I think
<cfbolz>
Great
<arigato>
test_zjit.py
<cfbolz>
mattip: this will probably take me a few days if you want to commit your mitigation patch that would be fine with me
<arigato>
(that's the main source of tests for greenfields, indirectly)
<cfbolz>
Yes, I see
<cfbolz>
arigato: I think there is a reason why removing the green field in interp_sre.py is enough to hide the bug. Only that file can lead to python -> sre -> python inlining
<arigato>
that sounds reasonable
inad922 has joined #pypy
solarjoe4 has quit [Quit: Leaving]
amaury_ has joined #pypy
energizer has quit [Ping timeout: 246 seconds]
oberstet has quit [Ping timeout: 240 seconds]
marr has joined #pypy
oberstet has joined #pypy
inad922 has quit [Ping timeout: 248 seconds]
Taggnostr2 has quit [Remote host closed the connection]
Taggnostr has joined #pypy
<kenaan>
mattip default d721da4573ad /pypy/module/_sre/interp_sre.py: workaround for issue 2225
inad922 has joined #pypy
<cfbolz>
mattip: that probably doesn't fix the other issue, does it?
<mattip>
2777? I doubt it (that is a match, not a sub), but will check
<mattip>
cfbolz: indeed the issue still exists. I started looking around at perhaps refactoring rpython/rlib/rsre/*.py to do the same trick,
<mattip>
is it worthwhile to simply avoid the issue? I would be happier if then we disallowed that field lookup in greens until we fix it
LooCfur has quit [Read error: Connection reset by peer]
pf_moore has joined #pypy
stduolc has left #pypy ["WeeChat 1.4"]
stduolc has joined #pypy
dddddd has quit [Read error: Connection reset by peer]
getxsick has joined #pypy
<arigato>
I'd leave it to cfbolz, it's probably not worth creating many conflicts
<kenaan>
cfbolz fix-sre-problems c93d31a2fabd /: stop using greenfields in sre, instead pass the pattern around
<kenaan>
cfbolz fix-sre-problems c553f2e29955 /: the context doesn't need the pattern any more
<cfbolz>
or rather in the switching from tracing to blackhole
<cfbolz>
aaaaaaaaaaaah
<cfbolz>
fijal: it's a SwitchToBlackhole coming from frontend_tag_overflow() in opencoder.py
<fijal>
pffffff
<fijal>
cfbolz: so my fault, to some extent?
<cfbolz>
yes :-)
<fijal>
sorry ;-)
<cfbolz>
np
<cfbolz>
it's amusing
<cfbolz>
so yayayayay, we have two different deep JIT bugs, both found with re in the same month!
<fijal>
our JIT is too complex
<cfbolz>
yes, but well
<cfbolz>
the alternatives are worse
<Alex_Gaynor>
fijal: our JIT is also the least complex one in it's performance class probably
<Alex_Gaynor>
At work they are currently working on making strings be allocated in the nursery, instead of always the old generation, and I'm not jealous of the mess.
<fijal>
Alex_Gaynor: ugh
marky1991 has joined #pypy
<simpson>
Alex_Gaynor: I agree, RPython's the least-complex JIT that I've seen the guts of and that also does what it does.
<gsnedders>
+1
<fijal>
that's a scary thought
<fijal>
we have also one of the smallest budgets
<Alex_Gaynor>
smallest budget might be a feature, we can't afford to do 10001 things manually, we have to come up with clever solutions
<simpson>
Huh, maybe Conway's Law applies and smaller budgets mean tighter code? As long as you can write tight code, I guess.
<fijal>
simpson: have you met armin? ;-)
<gsnedders>
Do any of you guys know how Graal compares, actually?
<simpson>
gsnedders: I like their design. Their API being Java means that it's horribly verbose, but the internals are probably very clean-looking in a UML viewer. My main concern is that SubstrateVM isn't open and Oracle isn't known for playing nice with anybody.
<simpson>
Also I'm not sure I like deopt in a partial-evaluation context. It's complex.
<gsnedders>
simpson: SubstrateVM has been GPLv2 for a few months now, FWIW
<gsnedders>
I wonder if Graal's complexity is getting worse since they got more budget :)
marr has joined #pypy
<cfbolz>
gsnedders: they can do a ton of nice things we can't afford, definitely
<simpson>
gsnedders: Huh. Hmmm. I am suspicious; Oracle releasing code under GPL does not necessarily mean that Oracle will not litigate in the future. But this is still promising and I didn't know about it, thanks.
<cfbolz>
but their python implementation is still early days, so unclear how they will compare
glyph has quit [Excess Flood]
glyph has joined #pypy
marky1991 has quit [Ping timeout: 264 seconds]
getxsick has quit [Ping timeout: 265 seconds]
marky1991 has joined #pypy
CountryNerd has joined #pypy
marky1991 has quit [Remote host closed the connection]
marky1991 has joined #pypy
raynold has joined #pypy
antocuni has joined #pypy
<kenaan>
cfbolz fix-sre-problems b3264f1f3592 /rpython/: fix issue 2777: before this commit, the following problem could occur: when the tags of the opencoder ov...
mcyprian has joined #pypy
oberstet has quit [Ping timeout: 264 seconds]
mcyprian has quit [Ping timeout: 256 seconds]
<cfbolz>
fijal: if you feel like taking a look, I'd be grateful
tbodt has joined #pypy
<fijal>
cfbolz: yeah I can
<fijal>
not sure how well would I swap things in :)
<cfbolz>
:-)
<cfbolz>
I'll run tests. I am fairly sure the commit or something like it is a good idea
<kenaan>
cfbolz fix-sre-problems fc18cb1d88b2 /rpython/: disable support for greenfields
<cfbolz>
just wondering whether there is a design consideration I missed