rosierui
commented
4 years ago I changed the following two lines, which resolved the issue. I sent a pull request, please check if the fix makes sense. Thanks, Jonathan
line 153 scope_names = self._scope_var_names.setdefault(scope_tuple, []) to scope_names = self._scope_var_names.setdefault(scope_tuple, set()) line 201 scope_names.append(name) to scope_names.add(name)
Thank you for the pyvcd project. I'm using pyvcd 0.1.4 with anconda3, python 3.7.3. Recently I found that VCDWriter::register_var() gives low performance when we have large number of variables, say > 5 millions, need to be register.The call becomes very slow,
https://github.com/SanDisk-Open-Source/pyvcd/blob/master/vcd/writer.py
Lines 153 - 155 def register_var(self, scope, name, var_type, size=None, init=None, ident=None): … Line # 153 scope_names = self._scope_var_names.setdefault(scope_tuple, []) 154 if name in scope_names: 155 raise KeyError('Duplicate var {} in scope {}'.format(name, scope))
Specifically line 154 appears perform linear search against scope_names which is a list, I did a test, it uses around 18'57" for 500,000 calls at line #154 cumtime1137.97539" (18'57") - writer.register_var step 3 - if name in scope_names: :called500,000 times, where scope_names has 499,426 items in it at the time.
Here is the test log for 500,000 calls $ grep "500,000" app.log cumtime 0.51248" - writer.register_var step 0 - check :called 500,000 times cumtime 0.9799" - writer.register_var step 1 - _get_scope_tuple :called 500,000 times cumtime 0.56376" - writer.register_var step 2 - _scope_var_names.setdefault :called 500,000 times cumtime 1137.97539" - writer.register_var step 3 - if name in scope_names: :called 500,000 times cumtime 0.79809" - writer.register_var step 4 - ident = format(id, 'x') :called 500,000 times cumtime 1.01963" - writer.register_var step 5 - var_str check :called 500,000 times cumtime 1.22152" - writer.register_var step 6 - var_str format :called 500,000 times cumtime 0.85593" - writer.register_var step 7 - *Variable :called 500,000 times cumtime 1.54725" - writer.register_var step 8 - change :called 500,000 times cumtime 0.92627" - writer.register_var step 9 - setdefault :called 500,000 times type(scope_names): <class 'list'>, len(scope_names): 499, 426,
Please confirm the finding is valid.