Skip to content

Commit

Permalink
In ASSIGN-POS, unbox the index just once
Browse files Browse the repository at this point in the history 
Measurements suggests this performs better, which is what would be
expected.
  • Loading branch information
@jnthn
jnthn committed Jan 10, 2018
1 parent 6e13bfa commit 86d91f2
Showing 1 changed file with 4 additions and 3 deletions.
7 changes: 4 additions & 3 deletions src/core/Array.pm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -483,17 +483,18 @@ my class Array { # declared in BOOTSTRAP
# Fast path: index > 0, $!reified is set up, either have a container
# or no $!todo so can just bind there
my \reified := nqp::getattr(self,List,'$!reified');
my int $ipos = $pos;
nqp::if(
nqp::isge_I($pos, 0) && nqp::isconcrete(reified),
nqp::isge_i($ipos, 0) && nqp::isconcrete(reified),
nqp::stmts(
(my \target := nqp::atpos(reified, $pos)),
(my \target := nqp::atpos(reified, $ipos)),
nqp::if(
nqp::isnull(target),
nqp::if(
nqp::isconcrete(nqp::getattr(self, List, '$!todo')),
self!ASSIGN-POS-SLOW-PATH($pos, assignee),
nqp::assign(
nqp::bindpos(reified, $pos, nqp::p6scalarfromdesc($!descriptor)),
nqp::bindpos(reified, $ipos, nqp::p6scalarfromdesc($!descriptor)),
assignee
)
),
Expand Down

2 comments on commit 86d91f2

@lizmat
Copy link
Contributor

@lizmat lizmat commented on 86d91f2 Jan 10, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW, I'm surprised

@jnthn
Copy link
Member Author

@jnthn jnthn commented on 86d91f2 Jan 10, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not in this commit, but I'll mention it: the code-gen for this ends up with the my \reified := nqp::getattr(self,List,'$!reified'); being lowered to a register, which is very helpful. The initial getatttr is guarded; prior to this it was guarded on multiple accesses, so there's some guard savings there (guards are cheap but not free).

The $ipos actually isn't register lowered to a register yet, but it's a native integer lexical, so there's no Scalar allocation that needs to happen. Since every routine has $/ and $! then we'd be allocating lexical storage anyway, so one more isn't really an extra cost, just an 8 byte bigger buffer. Further, the compilation is smart enough in this (easy) case to not acquire any lexical references. just to do cheap loads of the lexical, and they're all done by index (and that JITs very nicely). By contrast, an unbox has to do a bounds check to make sure the unboxed value will fit into a native integer, as well as the unbox itself. Pre-spesh, there's also deconts for $pos too before the unbox; spesh can rip those out, but speeding up the pre-specialization path is also nice.

Please sign in to comment.