Performance and simplicity #80

Closed
wants to merge 15 commits into from

2 participants

@pichi

Hi,
I have found some other places where code can be improved in readability and also in efficiency.

With best regards
Hynek Vychodil

pichi added some commits Nov 28, 2013
@pichi pichi Simplify get_pattern/2.
I had hard time figure out what was going out there. I think this code
is simpler to read and comprehend (and more efficient).
bce9295
@pichi pichi Simplify strip_types. b842e6f
@pichi pichi Simplify error and state handling in proper_typeserver:process_list/5 de6847c
@pichi pichi Remove hidden lists:reverse(X) ++ Y in convert_union/5 58c1fcd
@pichi pichi Improve efficiency of proper_arith:list_{remove, update, insert}
Original code was constructing list prefixes three times instead just once.
It affects garbage collection due memory consumption and also raw speed
a little bit.

Code changes raised function clause exception instead of badarg as
lists:split/2 does. I haven't seen any effect due only correct internal
usage.
fb10da9
@pichi pichi Simplify proper_statem:execute/4 to /3 (internal anyway) 3ff3b3a
@pichi pichi Simplify keep_shrinking/3 to /2. 2b2515e
@pichi pichi Simplify proper_arith:safe_map/2. c3ac191
@pichi pichi Simplify proper_arith:safe_zip/2. a3942ed
@pichi pichi Improve efficiency of proper_arith:partition/2 and proper_arith:filter/2
Traverse list in reverse order to avoid four or two reversing at the end.

Build only required lists for filter.
23ab316
@pichi pichi Simplify proper_arith:{remove/2, insert/3, unflatten/2}. 4747315
@pichi pichi Improve efficiency of jumble.
It changes O(N^2) to O(N*logN).
1efb9e8
@pichi pichi Inline proper_types:is_raw_type/1 the hottest function
It speeds up usage within module a little bit.
9a9f1e2
@pichi pichi Inline proper_gen:clean_instance/1 the second hottest function
It speeds up usage within module a little bit.
424b8d4
@pichi pichi Inline some hot and short functions in proper_types module. 6336e16
@kostis
Collaborator

Hi Hynek,

I've set aside this pull request, for quite a long while, partly due to being quite busy in other things but also because it is too big. In my opinion, it contains some very good code improvements and simplifications that definitely should be incorporated in the code of PropEr, and some others that are stylistic preferences that we can argue about whether they are code improvements or not.

But I will start cherry picking some of the changes and add them to the code base.

Thanks for your work!
Kostis

@pichi

OK, no problem.

@kostis kostis added a commit that referenced this pull request Jun 18, 2016
@kostis kostis Remove hidden lists:reverse(X) ++ Y in convert_union/5
Thanks to @pichi (#80)
f9291b8
@kostis kostis added a commit that referenced this pull request Jun 18, 2016
@kostis kostis Simplify proper_statem:execute/4 to /3 (internal anyway)
Thanks to @pichi (#80)
2073c87
@kostis kostis added a commit that referenced this pull request Jun 18, 2016
@kostis kostis Improve efficiency of proper_arith:partition/2 and proper_arith:filter/2
Traverse list in reverse order to avoid four or two reversing at the end.

Build only required lists for filter.

Thanks to @pichi (#80)
c38866b
@kostis kostis added a commit that referenced this pull request Jun 18, 2016
@kostis kostis Improve efficiency of jumble/1
Change complexity from O(N^2) to O(N*logN).

Thanks to @pichi (#80)
06ee374
@kostis kostis added a commit that referenced this pull request Jun 18, 2016
@kostis kostis Simplify is_raw_type/1 and is_raw_type_list/1
Also inline is_raw_type/1, the hottest function.

Thanks to @pichi (#80).
2e96238
@kostis kostis added a commit that referenced this pull request Jun 18, 2016
@kostis kostis Simplify clean_instance/1
Also add an inline declaration.  Thanks to @pichi (#80).
a96fd03
@kostis kostis added a commit that referenced this pull request Jun 18, 2016
@kostis kostis Inline some hot and short functions
Thanks to @pichi (#80).
8985c5b
@kostis
Collaborator

I have merged to master the following commits:

  • Remove hidden lists:reverse(X) ++ Y in convert_union/5 (58c1fcd)
  • Simplify proper_statem:execute/4 to /3 (internal anyway) (3ff3b3a)
  • Improve efficiency of proper_arith:partition/2 and proper_arith:filter/2 (23ab316)
  • Improve efficiency of jumble (1efb9e8)
  • Inline proper_types:is_raw_type/1 the hottest function (9a9f1e2)
  • Inline proper_gen:clean_instance/1 the second hottest function (424b8d4)
  • Inline some hot and short functions in proper_types module (6336e16)

The remaining ones, simplify the code a bit alright, but merely undo the extra work the authors of the code have put into writing tail recursive code. Indeed, such code is not the most beautiful to read or always faster than the non tail recursive version but it may have benefits when the recursion is deep.

Let me know if I missed anything important.

Once again, thanks for your work and an even bigger one for your patience!

@kostis
Collaborator

I will now close this pull request. Please open a new one if you think that some of the commits that were not included to master are important/useful and need to be part of it. Thanks again!

@kostis kostis closed this Jun 20, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment