Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Performance differences of record creation methods #425

Closed
JustinAzoff opened this issue Jun 19, 2019 · 0 comments · Fixed by #1199
Closed

Performance differences of record creation methods #425

JustinAzoff opened this issue Jun 19, 2019 · 0 comments · Fixed by #1199

Comments

@JustinAzoff
Copy link
Contributor

Given this record:

type Info: record {
    a: count;
    b: count;
    c: count;
};

And the following methods:

noop:

function foo() {
}

expanded:

function foo() {
    local tmp: Info;
    tmp$a=1;
    tmp$b=2;
    tmp$c=3;
}

list:

function foo() {
    local tmp: Info = [$a=1,$b=2,$c=3];
}

record:

function foo() {
    local tmp = Info($a=1,$b=2,$c=3);
}

record typed:

function foo(){
    local tmp: Info = Info($a=1,$b=2,$c=3);
}

and this driver script:

event zeek_init() {
    local x = 0;
    while ( ++x < 50000000 ) {
        foo();
    }
}

I see the following results:

##  record_inline/noop.bro
  50000000 took 12.07          4142427 it/sec
  50000000 took 11.83          4226811 it/sec
  50000000 took 11.99          4171400 it/sec
  50000000 took 11.90          4203419 it/sec
  50000000 took 11.86          4215261 it/sec
##  record_inline/expanded.bro
  50000000 took 25.20          1983888 it/sec
  50000000 took 25.45          1964645 it/sec
  50000000 took 24.95          2004054 it/sec
  50000000 took 25.22          1982687 it/sec
  50000000 took 24.96          2002827 it/sec
##  record_inline/list.bro
  50000000 took 32.99          1515566 it/sec
  50000000 took 30.32          1648815 it/sec
  50000000 took 29.68          1684754 it/sec
  50000000 took 29.94          1669781 it/sec
  50000000 took 30.67          1630465 it/sec
##  record_inline/record.bro
  50000000 took 38.16          1310325 it/sec
  50000000 took 42.22          1184264 it/sec
  50000000 took 44.39          1126294 it/sec
  50000000 took 37.63          1328741 it/sec
  50000000 took 38.24          1307655 it/sec
##  record_inline/record_typed.bro
  50000000 took 38.79          1288888 it/sec
  50000000 took 42.41          1179106 it/sec
  50000000 took 39.67          1260294 it/sec
  50000000 took 41.97          1191245 it/sec
  50000000 took 39.13          1277753 it/sec

It appears that once you account for the base function call overhead, creating records and then filling them in takes ~13s for 50000000, but using the typed record syntax takes 28s. I've seen this somewhat when looking at memory allocations as well, I believe the difference is because the 'nicer' syntax ends up allocating a temporary record that it has to merge into the final record.. so it takes 2x the time because it does 2x the allocations and 2x the work.

@timwoj timwoj self-assigned this Mar 4, 2020
@timwoj timwoj removed their assignment Jun 5, 2020
@rsmmr rsmmr self-assigned this Sep 30, 2020
rsmmr added a commit that referenced this issue Oct 5, 2020
We remove the inheritance from UnaryExpression because we know the
type of the operand precisely and can skip a temporary when evaluating
the expression.

#425
@jsiwek jsiwek closed this as completed in 553ce28 Oct 6, 2020
jsiwek added a commit that referenced this issue Oct 6, 2020
- Removed a now-unused-local-variable
- Added std::move() in AssignExpr::SetOp2()

* origin/topic/robin/gh-425-record-perf:
  Avoid unnecessary temporary value when coercing a record that's already the right type.
  Optimize record constructor expression.
  Unify type comparisions for records.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants