Allow to turn off stacktrace bookkeeping

[yonromai]

Hi,

The stack = traceback.extract_stack()[:-1] instruction makes inserts too slow for my use case.

This PR adds a flag to disable this behavior if needed.

According to some basic testing, it speeds up inserts by ~20x:

Test code:

from sqlitedict import SqliteDict

def insert(outer_stack):
    d = SqliteDict(outer_stack=outer_stack)
    for i in range(10000):
        d["key_{}".format(i)] = "value_{}".format(i)
    d.close()

%%timeit
insert(outer_stack=True)

=> 2.68 s ± 42.8 ms per loop (mean ± std. dev. of 7 runs, 1 loop each)

%%timeit
insert(outer_stack=False)

=> 121 ms ± 3.04 ms per loop (mean ± std. dev. of 7 runs, 10 loops each)

[piskvorky]

makes inserts too slow for my use case

Can you post some benchmarks? before / after your PR.

Intuitively, I'd expect debugging features that significantly slow down normal execution to be an opt-in, not opt-out. But if the difference is "slight", as per PR #28, it's OK as opt-out for power-users. So the benchmark numbers matter. CC @jquast .

[yonromai]

@mpenkov Thanks for the review - Amended the commit based on your comments, PTAL

[yonromai]

@piskvorky Although I cannot share my code (not open source), I observed a similar improvement as shown above (about ~23x faster after than before, although I didn't compute the variance in the real use case, as opposed to above).

FWIW I am evaluating this map compared to a leveldb index and sqlitedict is still ~4x slower than leveldb in my use case.

[piskvorky]

Yeah, that wouldn't surprise me. Sqlitedict is aiming to be a simple-to-use wrapper for SQLite, speed is not one of our primary objectives.

[yonromai]

@piskvorky LMK if you think I should make this the default behavior

[piskvorky]

@yonromai I'm not very familiar with this part of the code. Can you summarize the pros/cons?

What would we (the users, the developers) gain / lose by making it the default?

[yonromai]

Pro: Inserts are 23x faster.
Con: If an error occurs, the stack trace will mostly get swallowed, and 'No outer stack to display. Enable it using outer_stack=True' will be displayed in place of the stacktrace.

[piskvorky]

I'm not sure. I'm leaning slightly toward more intelligible messages… but damn, 23x is a lot! How often / under what conditions can these errors happen? In our code, in user code…?

@mpenkov WDYT?

'No outer stack to display. Enable it using outer_stack=True' will be displayed

That message is too vague: where should I (as a user) enter this outer_stack=True? Can you please make it more specific and actionable.

[mpenkov]

@yonromai why did you close this PR?

[yonromai]

@mpenkov I closed the PR because it's been stale and I'm trying to keep my github.com/pulls clean since I use it a lot.

I kept the fork alive in case you want to cherry pick the changes.

[mpenkov]

Are you able to complete the PR? You received feedback from @piskvorky. From what I can see:

1. Document the use case (most importantly, source of exceptions)
2. Improve error message

[yonromai]

@mpenkov Sorry but I don't have much more bandwidth to dedicate to this (we decided to not use sqlitedict).

[mpenkov]

OK, that's fair enough. Thanks for the changes in your PR. We'll take it from here.

[purplesyringa]

Has there been any progress on this PR? traceback.extract_stack() stat(2)'s all Python source files in the traceback--making more syscalls than sqlite itself. No wonder this is so slow.

[mpenkov]

@imachug No. Are you interested in fixing this?

[purplesyringa]

I thought this PR is more or less ready to merge, except maybe

That message is too vague: where should I (as a user) enter this outer_stack=True? Can you please make it more specific and actionable.

Do you have any other issues with this?

[mpenkov]

No, TBH, I think that's the only thing left. Let me go clean up the error message and we can merge this.

[piskvorky]

More context / info please. As a user, what is "the stacktrace at insert time", what are the implications of "not saving" it? Why would I want / not want that?