Allow start/stop recording exceptions in the coercion model

[robertwb]

This was detected in the following speed regression

In vanilla sage 4.6.2

sage: a=Integer(1000)
sage: b=int(1000)
sage: c=[0]
sage: %timeit c*a
625 loops, best of 3: 21.2 µs per loop
sage: %timeit c*b
625 loops, best of 3: 5.21 µs per loop

In vanilla sage 4.7.rc4

sage: a=Integer(1000)
sage: b=int(1000)
sage: c=[0]
sage: %timeit c*a
625 loops, best of 3: 568 µs per loop
sage: %timeit c*b
625 loops, best of 3: 5.13 µs per loop

By default the coercion model should not be recording exceptions ocurred in the execution of sage unless explicitely anabling reconding exceptions for debugging support.

Apply:

• 11388-coercion-speed.3.patch
• 11388-coercion-speed-referee-patch.patch

[jdemeyer]

Robert, I get a documentation warning, I would guess the docbuilder gets confused by the "\n" in the doctest:

docstring of sage.structure.coerce.CoercionModel_cache_maps.record_exceptions:11: (WARNING/2) Block quote ends without a blank line; unexpected unindent.

[robertwb]

Patch updated.

[lftabera]

mmm, I do not get any improvement. I get exactly the same timmings as in the ticket with the patch. I have tried on a fresh sage-4.7

Robert, has the issue dissapeared for you with this patch?

[robertwb]

OK, that's what comes of trying to add a patch in the middle of the night after a long day... it's missing a line. However, I think the fix at #11389 is better (rather than not record exceptions, we simply avoid them in many more cases).

[lftabera]

Ok, let me understand the code. There is a recording of exceptions that are very convenient for debugging. The problem is that it produces an important performance penalty. This is solved with record_exceptions() that controls the attribute _record_exceptions which is the flag that controls.

Just one thing. Each time that record_exceptions is called, the exception stack cached is cleared so:

sage: cm = sage.structure.element.get_coercion_model()
sage: cm.record_exceptions()
sage: cm._test_exception_stack()
sage: cm.record_exceptions()
sage: cm.exception_stack()
[]            

Is this the intended behavior? It is trivially seen from the code but I think it should be documented.

[jdemeyer]

Are you continueing with this ticket or should it be closed?

[lftabera]

No, this code is stull valid. The speed regression is gone, but the functions that control the record of the exceptions is still valid and worthy to include in sage.

[lftabera]

I give a positive review to the patch from Robert,

I just suggest some more verbosity in the documentation, as added in my patch.

[jhpalmieri]

The only thing needing reviewing is the referee patch, right? Positive review.