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

Lazy eventhandlers #1160

Merged
merged 3 commits into from
Jan 31, 2019
Merged

Lazy eventhandlers #1160

merged 3 commits into from
Jan 31, 2019

Conversation

xaviergonz
Copy link
Contributor

Performance optimization. does not create an EventHandlers object unless absolutely needed. In theory the ram gains should be bigger than the cpu gains.

after

-----------
Small Model
-----------
      31ms |      x 100 |  0.3ms avg
     182ms |    x 1,000 |  0.2ms avg
     794ms |   x 10,000 |  0.1ms avg
     163ms |    x 1,000 |  0.2ms avg
      15ms |      x 100 |  0.1ms avg

------------
Medium Model
------------
      34ms |      x 100 |  0.3ms avg
     325ms |    x 1,000 |  0.3ms avg
   1,661ms |   x 10,000 |  0.2ms avg
     295ms |    x 1,000 |  0.3ms avg
      43ms |      x 100 |  0.4ms avg

------------------------
Large Model - 0 children
------------------------
     110ms |      x 100 |  1.1ms avg
     665ms |    x 1,000 |  0.7ms avg
      97ms |      x 100 |  1.0ms avg

-------------------------------------------
Large Model - 10 small & 10 medium children
-------------------------------------------
      99ms |       x 50 |  2.0ms avg
     336ms |      x 250 |  1.3ms avg
      98ms |       x 50 |  2.0ms avg

-------------------------------------------
Large Model - 100 small & 0 medium children
-------------------------------------------
     113ms |       x 50 |  2.3ms avg
     314ms |      x 250 |  1.3ms avg
      51ms |       x 50 |  1.0ms avg

-------------------------------------------
Large Model - 0 small & 100 medium children
-------------------------------------------
     342ms |       x 50 |  6.8ms avg
   1,267ms |      x 250 |  5.1ms avg
     325ms |       x 50 |  6.5ms avg

20.60user 4.18system 0:11.89elapsed 208%CPU (0avgtext+0avgdata 264200maxresident)k
0inputs+0outputs (0major+197978minor)pagefaults 0swaps
Done in 42.04s.

before

-----------
Small Model
-----------
      34ms |      x 100 |  0.3ms avg
     177ms |    x 1,000 |  0.2ms avg
     838ms |   x 10,000 |  0.1ms avg
     170ms |    x 1,000 |  0.2ms avg
      12ms |      x 100 |  0.1ms avg

------------
Medium Model
------------
      41ms |      x 100 |  0.4ms avg
     363ms |    x 1,000 |  0.4ms avg
   1,804ms |   x 10,000 |  0.2ms avg
     308ms |    x 1,000 |  0.3ms avg
      37ms |      x 100 |  0.4ms avg

------------------------
Large Model - 0 children
------------------------
     114ms |      x 100 |  1.1ms avg
     696ms |    x 1,000 |  0.7ms avg
      83ms |      x 100 |  0.8ms avg

-------------------------------------------
Large Model - 10 small & 10 medium children
-------------------------------------------
     108ms |       x 50 |  2.2ms avg
     365ms |      x 250 |  1.5ms avg
     119ms |       x 50 |  2.4ms avg

-------------------------------------------
Large Model - 100 small & 0 medium children
-------------------------------------------
      88ms |       x 50 |  1.8ms avg
     343ms |      x 250 |  1.4ms avg
      90ms |       x 50 |  1.8ms avg

-------------------------------------------
Large Model - 0 small & 100 medium children
-------------------------------------------
     358ms |       x 50 |  7.2ms avg
   1,384ms |      x 250 |  5.5ms avg
     328ms |       x 50 |  6.6ms avg

21.26user 5.35system 0:12.59elapsed 211%CPU (0avgtext+0avgdata 260288maxresident)k
0inputs+0outputs (0major+206645minor)pagefaults 0swaps
Done in 43.47s.

@xaviergonz xaviergonz requested a review from k-g-a January 30, 2019 22:17
@xaviergonz
Copy link
Contributor Author

@k-g-a would you be so kind as to test it against your playground that you showed in #1151 (comment) ?

@k-g-a
Copy link
Member

k-g-a commented Jan 31, 2019

@xaviergonz, with pleasure. I'll try to spare some time tomorrow.

@k-g-a
Copy link
Member

k-g-a commented Jan 31, 2019

Made a quick glance: 550Kb out of 7.8Mb are wiped out, thats around 7%.
image
image

That's a good cut as many users payed for nothing.
But if I understand correclty, should I use safe reference for my GrandChild (deepest nodes) - those Kbs will come back?

Copy link
Member

@k-g-a k-g-a left a comment

Choose a reason for hiding this comment

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

No questions considering laziness. I will try to measure the situation when EventHandlers come back in game and try to optimize a bit. Good old functions instead of classes, or flatter heirarchy maybe - not sure for now.
This one should be merged, great work! :)

@xaviergonz
Copy link
Contributor Author

xaviergonz commented Jan 31, 2019

But if I understand correclty, should I use safe reference for my GrandChild (deepest nodes) - those Kbs will come back?

Yep, if a hook is attached (such as what happens when using safe reference) then those KBs will come back, but I don't see many ways around that one :)

PS: this is what I meant on the proposal when I said that adding hooks to scalar nodes shouldn't really hurt that much

@xaviergonz xaviergonz merged commit 4290616 into master Jan 31, 2019
@xaviergonz xaviergonz deleted the lazy-eventhandlers branch February 2, 2019 09:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants