2
\$\begingroup\$

This is a continuation from my original question. After the improvements suggested by @Reinderien and some of my own.
This approach I've taken is kinda obvious. And I'm not using any parallel processing. I think there's scope for improvement because I know of a crate, Rayon in Rust which could have run the steps I'm currently running, parallelly. I'll explain why I think this is possible below.

"""
Find the number of 'exceptions' and 'added' event's in the exception log
with respect to the device ID.

author: clmno
date: 2018-12-23
updated: 2018-12-27
"""

from time import time
import re

def timer(fn):
    """ Used to time a function's execution"""
    def f(*args, **kwargs):
        before = time()
        rv = fn(*args, **kwargs)
        after = time()
        print("elapsed", after - before)
        return rv
    return f

#compile the regex globally
re_prefix = '.*?'
re_guid='([A-Z0-9]{8}-[A-Z0-9]{4}-[A-Z0-9]{4}-[A-Z0-9]{4}-[A-Z0-9]{12})'
rg = re.compile(re_prefix+re_guid, re.IGNORECASE|re.DOTALL)

def find_sql_guid(txt):
    """ From the passed in txt, find the SQL guid using re"""
    m = rg.search(txt)
    if m:
        guid1 = m.group(1)
    else:
        print("ERROR: No SQL guid in line. Check the code")
        exit(-1)
    return guid1

@timer
def find_device_IDs(file_obj, element):
    """ Find the element (type: str) within the file (file path is 
        provide as arg). Then find the SQL guid from the line at hand.
        (Each line has a SQL guid)
        Return a dict of {element: [<list of SQL guids>]}
    """
    lines = set()
    for line in file_obj:
        if element in line:
            #find the sql-guid from the line-str & append
            lines.add(find_sql_guid(line))
    file_obj.seek(0)
    return lines

@timer
def find_num_occurences(file_obj, key, search_val, unique_values):
    """ Find and append SQL guids that are in a line that contains a string
        that's in search_val into 'exception' and 'added'
        Return a dict of {'exception':set(<set of SQL guids>), 
                        'added': set(<set of SQL guids>)}
    """
    lines = {'exception':set(), 'added': set()}

    for line in file_obj:
        for value in unique_values:
            if value in line:
                if search_val[0] in line:
                    lines['exception'].add(value)
                elif search_val[1] in line:
                    lines['added'].add(value)
    file_obj.seek(0)
    return lines

def print_stats(num_exceptions_dict):
    for key in num_exceptions_dict.keys():
        print("{} added ".format(key) + 
            str(len(list(num_exceptions_dict[key]["added"]))))
        print("{} exceptions ".format(key) + 
            str(len(list(num_exceptions_dict[key]["exception"]))))

if __name__ == "__main__":
    path  = 'log/server.log'
    search_list = ('3BAA5C42', '3BAA5B84', '3BAA5C57', '3BAA5B67')

    with open(path) as file_obj:
        #find every occurance of device ID and find their corresponding SQL
        # guids (unique ID)
        unique_ids_dict = {
            element: find_device_IDs(file_obj, element)
            for element in search_list
        }

        #Now for each unique ID find if string ["Exception occurred", 
        # "Packet record has been added"] is found in it's SQL guid list.
        search_with_in_deviceID = ("Exception occurred", 
                                    "Packet record has been added")
        #reset the file pointer
        file_obj.seek(0)
        num_exceptions_dict = {
            elem: find_num_occurences(file_obj, elem, search_with_in_deviceID,
                                    unique_ids_dict[elem])
            for elem in search_list
        }

    print_stats(num_exceptions_dict)

and here's a small server log for you to experiment on

Improvements

  • More pythonic with some help from Reinderien.
  • Opening the file only once. Can't see a significant change in execution speed.
  • Using better data structure model. Was using dicts everywhere, sets made sense.

My current approach is to

  1. Find the device ID (eg 3BAA5C42) and their corresponding SQL GUIDs.
  2. For each SQL GUID find if it resulted in an exception or added event. Store them in dict.
  3. Print the stats

Parallelize
Step one and two are just going through the file searching for a particular string and performing a set of instructions once the sting is found. And so each process (both within steps one and two, and step one and two as a whole) is independent of each other / mutually exclusive. And so running them in parallel makes more sense.

How should I get going to improve this code?

\$\endgroup\$
  • \$\begingroup\$ And I've read of Donald Knuth's famour string search algo Knuth-Morris-Pratt. What I'm doing is a string search. Just wondering \$\endgroup\$ – clmno Dec 27 '18 at 6:36
  • \$\begingroup\$ Your dictionary comprehension to find the unique IDs does not work. After find_device_IDs has run for the first time, you have reached the end of the file, so all except one ID are empty sets. A file_obj.seek(0) at the end of the function would help for now. \$\endgroup\$ – Graipher Dec 27 '18 at 10:03
  • \$\begingroup\$ The signature of that function should also be def find_device_IDs(file_obj, element):, otherwise it just accesses the global variable file_obj. \$\endgroup\$ – Graipher Dec 27 '18 at 10:08
  • \$\begingroup\$ @Graipher Fixed that in the code and tried running again. Sadly, there isn't a significant improvement from my previous version. (its 45s now, used to be 47s) \$\endgroup\$ – clmno Dec 27 '18 at 10:22
  • 1
    \$\begingroup\$ Will write an answer to see if you can get away with a single pass over the file. \$\endgroup\$ – Graipher Dec 27 '18 at 10:25
3
\$\begingroup\$

As evidenced by your timings a major bottleneck of your code is having to read the file multiple times. At least it is now only opened once, but the actual content is still read eight times (once for each unique ID, so four times in your example, and then once again each for the exceptions).

First, let's reduce this to two passes, once for the IDs and once for the exceptions/added events:

from collections import defaultdict

@timer
def find_device_IDs(file_obj, search_list):
    """ Find the element (type: str) within the file (file path is 
        provide as arg). Then find the SQL guid from the line at hand.
        (Each line has a SQL guid)
        Return a dict of {element: [<list of SQL guids>]}
    """
    sql_guids = defaultdict(set)
    for line in file_obj:
        for element in search_list:
            if element in line:
                #find the sql-guid from the line-str & append
                sql_guids[element].add(find_sql_guid(line))
    return sql_guids

The exception/added finding function is a bit more complicated. Here we first need to invert the dictionary:

device_ids = {sql_guid: device_id for device_id, values in unique_ids_dict.items() for sql_guid in values}
# {'0af229d1-283e-4575-a818-901617a762a7': '3BAA5C57',
#  '2f4a7f93-d7ed-4514-bef0-9bb0f025ecd3': '3BAA5C42',
#  '4e720c6e-1866-4c9b-b967-dfab049266fb': '3BAA5B67',
#  '85708e5d-768d-4a90-ab71-60a737de96e3': '3BAA5B67',
#  'e268b224-bfb7-40c7-8ae5-500eaecb292b': '3BAA5B84',
#  'e4ced298-530c-41cc-98a7-42a2e4fe5987': '3BAA5B67'}

Then we can use that:

@timer
def find_num_occurences(file_obj, sql_guids, search_vals):
    device_ids = {sql_guid: device_id for device_id, values in sql_guids.items() for sql_guid in values}
    data = defaultdict(lambda: defaultdict(set))

    for line in file_obj:
        for sql_guid, device_id in device_ids.items():
            if sql_guid in line:
                for key, search_val in search_vals.items():
                    if search_val in line:
                        data[device_id][key].add(sql_guid)
    return data

The usage is almost the same as your code:

with open(path) as file_obj:
    device_ids = ('3BAA5C42', '3BAA5B84', '3BAA5C57', '3BAA5B67')
    sql_guids = find_device_IDs(file_obj, device_ids)
    file_obj.seek(0)

    search_with_in_deviceID = {"exception": "Exception occurred", 
                               "added": "Packet record has been added"}
    print(find_num_occurences(file_obj, sql_guids, search_with_in_deviceID))

# defaultdict(<function __main__.find_num_occurences.<locals>.<lambda>>,
#             {'3BAA5B67': defaultdict(set,
#                          {'added': {'4e720c6e-1866-4c9b-b967-dfab049266fb'},
#                           'exception': {'85708e5d-768d-4a90-ab71-60a737de96e3',
#                            'e4ced298-530c-41cc-98a7-42a2e4fe5987'}}),
#              '3BAA5B84': defaultdict(set,
#                          {'added': {'e268b224-bfb7-40c7-8ae5-500eaecb292b'}}),
#              '3BAA5C42': defaultdict(set,
#                          {'added': {'2f4a7f93-d7ed-4514-bef0-9bb0f025ecd3'}}),
#              '3BAA5C57': defaultdict(set,
#                          {'added': {'0af229d1-283e-4575-a818-901617a762a7'}})})

You can actually get this down to a single pass, by collecting all IDs where an exception occurred and only at the end joining that with the elements you are actually searching for:

def get_data(file_obj, device_ids, search_vals):
    sql_guid_to_device_id = {}
    data = defaultdict(set)

    for line in file_obj:
        # search for an sql_guid
        m = rg.search(line)
        if m:
            sql_guid = m.group(1)

            # Add to mapping
            for device_id in device_ids:
                if device_id in line:
                    sql_guid_to_device_id[sql_guid] = device_id

            # Add to exceptions/added
            for key, search_val in search_vals.items():
                if search_val in line:
                    data[sql_guid].add(key)
    return sql_guid_to_device_id, data

def merge(sql_guid_to_device_id, data):
    data2 = defaultdict(lambda: defaultdict(set))

    for sql_guid, values in data.items():
        if sql_guid in sql_guid_to_device_id:
            for key in values:
                data2[sql_guid_to_device_id[sql_guid]][key].add(sql_guid)
    return data2

With the following usage:

with open(path) as file_obj:
    device_ids = ('3BAA5C42', '3BAA5B84', '3BAA5C57', '3BAA5B67')
    search_with_in_deviceID = {"exception": "Exception occurred", 
                               "added": "Packet record has been added"}
    sql_guid_to_device_id, data = get_data(file_obj, device_ids, search_with_in_deviceID)
    data2 = merge(sql_guid_to_device_id, data)

    for device_id, values in data2.items():
        for key, sql_guids in values.items():
            print(f"{device_id} {key} {len(sql_guids)}")

# 3BAA5B67 exception 2
# 3BAA5B67 added 1
# 3BAA5C42 added 1
# 3BAA5B84 added 1
# 3BAA5C57 added 1

get_data, data and data2 still need better names...

Other than that this should be faster because it reads the file only once. It does consume more memory, though, because it also saves exceptions or added events for SQL guids which you later don't need. If this trade-off is not worth it, go back to the first half of this answer.

\$\endgroup\$
  • 2
    \$\begingroup\$ The one pass solution was pretty awesome! It has brought it down to 6 seconds! TIL Lesser IO operations, faster your code. Coming from hardware/fw we always use IOs and rarely have enough RAM. ty \$\endgroup\$ – clmno Dec 27 '18 at 12:18

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.