take transducer optimization

Description

A basic refactoring to remove the let form and only requires a single counter check for each iteration, yields an 25% performance increase. With the patch, 2 checks are only required for the last iteration (in case counter arg was <= 0)...

Environment

None

Activity

Show:
Karsten Schmidt
August 26, 2015, 12:01 PM

Good idea, Alex! This 2nd patch removes the neg? check and adds a quick bail transducer for cases where n <= 0. It also made it slightly faster still:

(now ~35% faster than original)

Alex Miller
August 26, 2015, 1:22 AM

The only time that neg? case will be hit is if take is passed n <= 0, so I think this is correct. However, you might consider some different way to handle that particular case - for example, it could be pulled out of the transducer function entirely and be created as a separate transducer function entirely. I'm not sure that's worth doing, but it's an idea.

Karsten Schmidt
August 25, 2015, 5:58 PM

Hi Alex, try running the tests... AFAICT it's all still working as advertised: For (take 0) or (take -1) then the pos? check fails, but we must ensure to not call rf for that iteration. For all other (take n) cases only the pos? check is executed apart from the last iteration (which causes a single superfluous neg? call) The current path/impl always does 2 checks for all iterations and hence is (much) slower.

Alex Miller
August 25, 2015, 5:28 PM

From a superficial skim, I see checks for pos? and then neg? which both exclude 0 and that gives me doubts about that branch. That may actually be ok but I will have to read it more closely.

Karsten Schmidt
August 25, 2015, 4:35 PM

Proposed patch, passes all existing tests

Your pinned fields
Click on the next to a field label to start pinning.

Assignee

Unassigned

Reporter

Karsten Schmidt

Approval

Triaged

Patch

Code

Priority

Minor

Affects versions