byuccl / bfasst

Tools for FPGA Assurance Flows
Apache License 2.0
12 stars 5 forks source link

Speed up Comparison #497

Closed yonnorc42 closed 3 months ago

yonnorc42 commented 3 months ago

I'm starting this pull request now but it's going to be a draft for a while until I've figured out exactly what changes we want to make and how to make them.

Current changes:

yonnorc42 commented 3 months ago

One of the changes I've made might dramatically slow down a run the first time, but should increase the speed in subsequent runs since the list of possible matches for each instance should hopefully be significantly shorter so comparison will be faster and the caching will be faster... as long as I get it working.

yonnorc42 commented 3 months ago

While using the cache, the runtime of jpegencode through comparison is about 16 minutes.

reillymck commented 3 months ago

I was thinking about what you said in the meeting about the dictionary lookup times to for {instance_name: instance} dictionary. Since this datatype does not change, we should use a list. The potential matching sets should just have the list indices for the master instances list (instead of using the instance name keys).. Python list access should be O(1) What do you think?

yonnorc42 commented 3 months ago

Looking at the profile, I don't think it is actually taking as long as I thought. It was only a quarter of a second in the entire jpegencode run. @reillymck 3244748 0.246 0.000 0.246 0.000 {method '__getitem__' of 'dict' objects}

Edit: Actually this might be the line in the profile:

1500319645 103.563 0.000 103.563 0.000 {method 'get' of 'dict' objects}

I'm not using the get method, just the built in access method, so I'm not sure which one of these it actually is.

The only time get is used on dicts is a line which was already in the code previous to any of my changes.

yonnorc42 commented 3 months ago

Also, from what I'm seeing online, dict access is also about O(1)

yonnorc42 commented 3 months ago

Going to push a few minor netlist_cleanup changes as well, same logic slightly different format

yonnorc42 commented 3 months ago

Just added the flow argument logging_level to work with these tools: netlist_cleanup, error injector, phys netlist, and structural comparison.

Example flow run:

python scripts/run.py VivadoPhysNetlistCmp --flow_arguments "{'logging_level': 'ERROR'}"

Will only log messages with logging.error or above, so no logging.debug, logging.info, or logging.warning messages will get added to the log file. The options are DEBUG, INFO, WARNING, ERROR, CRITICAL

yonnorc42 commented 3 months ago

This should be good to merge @reillymck or we can talk about it during the meeting if there's anything else to be added