Comment by eesmith | Hacker News Reader

Comment by eesmith | Hacker News Reader

The parse code at https://github.com/gregstoll/baseballstats/blob/443dbefe1d9b... shows a few ways the Python code could be sped up:

        if (not doneParsingEvent):
            if (batterEvent.startswith('K')):
                runnerDests['B'] = 0
                runnersDefaultStayStill = True
                if (batterEvent.startswith('K+') or batterEvent.startswith('K23+')):
                    if batterEvent.startswith('K+'):

The "not doneParsingEvent" is used to emulate a "continue" statement. As is, every line can trigger a large number if "if (not doneParsing):" tests.

Python's s.startswith() methods are very attractive, but slow compared to the error-prone:

    if batterEvent[0] == "K":
      ...
      if batterEvent[:2] == "K+":
         ..

Here are the timings:

    % python -m timeit -s 's = "Knights"' 's.startswith("K")'
    2000000 loops, best of 5: 119 nsec per loop
    % python -m timeit -s 's = "Knights"' 's[:1] == "K"'
    5000000 loops, best of 5: 76.5 nsec per loop
    % python -m timeit -s 's = "Knights"' 's[0] == "K"'
    5000000 loops, best of 5: 41.4 nsec per loop

There are a number of uses of:

    simpleHitMatch = getRe(r"^([SDTH])(?:\d|/)").match(batterEvent)

where "getRe()" implements a simple regex cache. However 1) re.match() also implements a simple regex cache, and 2) I typically implement this as a per-module variable, one per regex:

    _simple_hit_pattern_matcher = re.compile(r"^([SDTH])(?:\d|/)").match

which is then used as:

    simpleHitMatch = _simple_hit_pattern_matcher(batterEvent)

This would reduce the call overhead by removing the double cache table lookup.

Bear in mind that I don't know where the time is spent (so I may be looking in the wrong place) and I don't think these changes could get an 8x performance improvement.

OP here. I went ahead and tried using the slicing instead of startswith() a few places, and moving the regexes out to module variables. (thanks for the suggestions!) It did speed up, but just a little bit (from ~32 secs to ~31 secs).

I may try changing the "not doneParsingEvent" checks into something that uses "continue", I'd imagine that would probably help. (although the Rust code is structured the same way) Another thing I've considered is looking at the data and figuring out which cases are more common and checking for those first...

Another thing I considered is

I read through your previous posts.

I think the double caching is why "some more cleanup to cache compiled regular expressions that I thought would also speed things up a little" didn't actually speed things up - it was already cached. (See Python's re.py in def _compile(): where it uses '_cache'.)

Did you post something where you profiled your code? I didn't see it. The cprofile module gives a decent first-pass at figuring out bottlenecks, and it's sometimes not what you think it is.

If I find that the time is spent in one or a handful of functions, and the slow-down still isn't obvious, I'll then use line_profiler for more fine-grained profiling.

There are other profiling tools, but I haven't need to use them for many years for my current projects.

Yup, that makes a lot of sense - I didn't realize Python cached so many previous regex's, which is nice!

I did use cprofile a while back and I think at the time it showed a lot of time in the regexp matching. (I don't think I posted about it though) Honestly, once I implemented the parallelism I got less motivated to make it faster. But now I am kinda curious what the speed difference is between Rust and Python if I spend time trying to optimize both.

Thanks for the suggestions!

Rust should be a lot faster than Python if your time is mostly spent parsing the contents of those lines. Think that each Python op-code will be running extra assembly instructions, just to handle the virtual machine overhead.

I see a number of micro-optimizations that may give you a few percent more in Python.

For example, you use GameSituation as a mutable way to maintain parse state. You modify it with things like "gameSituation.outs += 1".

Mutating instance attributes has a much higher overhead in CPython than in C/C++ (and presumably Rust). You can reduce some of that overhead by telling the class which slots to have.

Consider "spam.py" containing the following:

  class Foo:
    def __init__(self):
      self.a = 0

  class Bar(Foo):
    __slots__ = ("a",)

  % python -m timeit -s 'import spam; x=spam.Foo()' 'x.a = 3'
  5000000 loops, best of 5: 41 nsec per loop
  % python -m timeit -s 'import spam; x=spam.Bar()' 'x.a = 3'
  10000000 loops, best of 5: 32.4 nsec per loop

If you replace your GameSituation with a dict then you can get a little faster, but not enough to worry about.

  % python -m timeit -s 'import spam; x={}' 'x["a"] = 3'

10000000 loops, best of 5: 29 nsec per loop

Another micro-optimization is to reduce the number of temporary strings. Consider:

     if (batterEvent.startswith('W+') or batterEvent.startswith('IW+') or batterEvent.startswith('I+')):
        tempEvent = batterEvent[2:]

If you track the current offset in the string, then you can do things like:

     if batterEvent[i:i+2] in ("W+", "I+") or batterEvent[i:i+3] == "IW+":
        i += 2

and use the start position parameter in the re.match() calls.

(BTW, there appears to be a bug in your original code, since "IW+" is 3 letters long.)

Another BTW, you might change "for line in f.readlines()" to "for line in f". Shouldn't affect performance but should reduce your overall memory use.

In closing, character-level string processing in CPython is slow so I doubt you'll get all that much faster.

You might try pypy, but with the number of temporary strings you create, my guess is pypy still won't be that much faster. Should be easy to test though.

Reply on news.ycombinator.com