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

mod_profiling - wrong function prototype for on_punch #4220

Closed
JDCodeIt opened this issue Jun 14, 2016 · 7 comments
Closed

mod_profiling - wrong function prototype for on_punch #4220

JDCodeIt opened this issue Jun 14, 2016 · 7 comments
Labels
Bug Issues that were confirmed to be a bug

Comments

@JDCodeIt
Copy link
Contributor

JDCodeIt commented Jun 14, 2016

Turning on profiling, then punching an entity causes crash for mods that expect 'dir" to not be nil as specified in the API. This seems to be because the function does not pass enough parameters with the profiling substitute.

builtin/game/mod_profiling.lua:290

            new_on_punch = function(self, hitter)
                local starttime = core.get_us_time()
mod_statistics.entity_callbacks[cbid](self, hitter)

should that not be

on_punch(self, hitter, time_from_last_punch, tool_capabilities, dir)
and
mod_statistics.entity_callbacks[cbid](self, hitter, time_from_last_punch, tool_capabilities, dir)

@t4im
Copy link
Contributor

t4im commented Jun 21, 2016

This is actually a known issue, and will be fixed with #3849
I just noticed the decision mentioned therein myself recently, due to a longer off-period, but gonna try to look into merging that soon, when i get a free minute. Until then, you might be able to use https://github.com/t4im/profiler/ as a temporary drop-in replacement.

@t4im
Copy link
Contributor

t4im commented Jun 22, 2016

Please test #4245 and report back any issues.

@paramat paramat added the Bug Issues that were confirmed to be a bug label Jun 22, 2016
@JDCodeIt
Copy link
Contributor Author

Hi Tim, downloaded and compiled this on Linux Xubuntu 14.04 LTS. It
seems to work fine on the tests I did with many mods loaded.

There were a lot of zero's on the % columns so I changed it to show one
decimal.

reporter.lua Line 54: self:print(" %-55s | %9d | %9d | %9d | %5d
| %5.1f | %5.1f",

I tested /profile dump, print, save, and reset.

See the attached "save" result - those zombies are hungry!

On 06/22/2016 10:08 AM, Tim wrote:

Please test #4245 #4245 and
report back any issues.


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#4220 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/ALoEtH2dmTggEbzV9xKwm9Tsu3xc0H4Pks5qOV4SgaJpZM4I1c0S.

Values below show times/percentages per server step.
A total of 1927 samples were taken

instrumentation | min µs | max µs | avg µs | min % | max % | avg %
---------------------------------------------------------+-----------+-----------+-----------+-------+-------+-------
node_rental: | 0 | 283 | 1 | 0 | 48.6 | 0.3

  • globalstep GlowStone code #1 ...................................... | 0 | 283 | 1 | 0 | 48.6 | 0.3
    areas: | 5 | 3092 | 33 | 0 | 90.8 | 7.6
  • globalstep GlowStone code #1 ...................................... | 5 | 3092 | 33 | 0 | 90.8 | 7.6
    sprint: | 0 | 1666 | 12 | 0 | 77.6 | 1.9
  • globalstep GlowStone code #1 ...................................... | 0 | 1666 | 12 | 0 | 77.6 | 1.9
    farming: | 0 | 141 | 0 | 0 | 8.9 | 0.0
  • ABM Furnace segfault #3 ............................................. | 0 | 141 | 0 | 0 | 8.9 | 0.0
    seacoral: | 0 | 147 | 0 | 0 | 19.7 | 0.1
  • ABM Fix build with Pathscale's EKOPath #7 ............................................. | 0 | 32 | 0 | 0 | 5.2 | 0.0
  • ABM Burned wood #2 ............................................. | 0 | 131 | 0 | 0 | 17.5 | 0.0
  • ABM Fixed key names so the key set menu now works. #5 ............................................. | 0 | 69 | 0 | 0 | 5.8 | 0.0
  • ABM Saplings + Growing Trees #8 ............................................. | 0 | 6 | 0 | 0 | 0.4 | 0.0
  • ABM Fixed null pointer dereference errors found by cppcheck #10 ............................................ | 0 | 22 | 0 | 0 | 4.1 | 0.0
  • ABM Furnace segfault #3 ............................................. | 0 | 18 | 0 | 0 | 3.0 | 0.0
  • ABM Apples on the trees can not be eaten #6 ............................................. | 0 | 29 | 0 | 0 | 2.5 | 0.0
  • ABM Only use gettext when it's enabled #11 ............................................ | 0 | 7 | 0 | 0 | 1.8 | 0.0
  • ABM OK, Saplings For Real #9 ............................................. | 0 | 14 | 0 | 0 | 2.5 | 0.0
  • ABM GlowStone code #1 ............................................. | 0 | 16 | 0 | 0 | 0.5 | 0.0
  • ABM cppcheck warnings #4 ............................................. | 0 | 97 | 0 | 0 | 5.2 | 0.0
  • ABM Jack O Lantern for Halloween ! #12 ............................................ | 0 | 9 | 0 | 0 | 0.9 | 0.0
    creatures: | 38 | 4973 | 130 | 0 | 86.8 | 27.7
  • ABM Furnace segfault #3 ............................................. | 0 | 28 | 0 | 0 | 1.5 | 0.0
  • ABM Burned wood #2 ............................................. | 0 | 8 | 0 | 0 | 0.4 | 0.0
  • [creatures:zombie].on_step() ....................... | 38 | 4973 | 130 | 0 | 86.8 | 27.7
  • [creatures:zombie].on_activate() ................... | 0 | 35 | 0 | 0 | 2.5 | 0.0
  • [creatures:zombie].get_staticdata() ................ | 0 | 42 | 0 | 0 | 3.0 | 0.0
  • ABM GlowStone code #1 ............................................. | 0 | 385 | 0 | 0 | 27.2 | 0.0
    unified_inventory: | 3 | 1745 | 18 | 0 | 80.4 | 3.9
  • globalstep GlowStone code #1 ...................................... | 3 | 1745 | 18 | 0 | 80.4 | 3.9
    ambience: | 0 | 2035 | 25 | 0 | 74.3 | 3.3
  • globalstep GlowStone code #1 ...................................... | 0 | 2035 | 25 | 0 | 74.3 | 3.3
    pipeworks: | 0 | 3181 | 8 | 0 | 74.6 | 1.5
  • globalstep GlowStone code #1 ...................................... | 0 | 3179 | 6 | 0 | 74.4 | 0.9
  • globalstep Burned wood #2 ...................................... | 0 | 322 | 2 | 0 | 22.9 | 0.6
    kpgmobs: | 15 | 6775 | 72 | 0 | 86.5 | 13.5
  • [kpgmobs:rat].on_step() ............................ | 0 | 129 | 2 | 0 | 25.9 | 0.7
  • [kpgmobs:bee].on_activate() ........................ | 0 | 31 | 0 | 0 | 3.0 | 0.0
  • ABM Burned wood #2 ............................................. | 0 | 279 | 0 | 0 | 26.9 | 0.0
  • ABM Furnace segfault #3 ............................................. | 0 | 1 | 0 | 0 | 0.2 | 0.0
  • [kpgmobs:bee].on_step() ............................ | 2 | 620 | 9 | 0 | 37.2 | 1.9
  • ABM Only use gettext when it's enabled #11 ............................................ | 0 | 222 | 0 | 0 | 15.2 | 0.0
  • ABM cppcheck warnings #4 ............................................. | 0 | 2 | 0 | 0 | 0.2 | 0.0
  • [kpgmobs:horse2].on_step() ......................... | 2 | 637 | 11 | 0 | 63.5 | 2.3
  • [kpgmobs:cow].on_activate() ........................ | 0 | 26 | 0 | 0 | 1.8 | 0.0
  • ABM Fixed key names so the key set menu now works. #5 ............................................. | 0 | 7 | 0 | 0 | 0.4 | 0.0
  • [kpgmobs:bee].get_staticdata() ..................... | 0 | 37 | 0 | 0 | 3.6 | 0.0
  • ABM Fixed null pointer dereference errors found by cppcheck #10 ............................................ | 0 | 811 | 0 | 0 | 3.0 | 0.0
  • [kpgmobs:medved].on_step() ......................... | 2 | 442 | 3 | 0 | 59.8 | 0.8
  • [kpgmobs:cow].on_step() ............................ | 8 | 6178 | 44 | 0 | 85.7 | 7.9
  • ABM Saplings + Growing Trees #8 ............................................. | 0 | 1 | 0 | 0 | 0.2 | 0.0
  • ABM GlowStone code #1 ............................................. | 0 | 8 | 0 | 0 | 0.4 | 0.0
  • [kpgmobs:medved].on_activate() ..................... | 0 | 96 | 0 | 0 | 0.4 | 0.0
  • [kpgmobs:cow].get_staticdata() ..................... | 0 | 27 | 0 | 0 | 1.9 | 0.0
  • [kpgmobs:medved].get_staticdata() .................. | 0 | 276 | 0 | 0 | 1.0 | 0.0
    wieldview: | 0 | 343 | 1 | 0 | 31.2 | 0.5
  • globalstep GlowStone code #1 ...................................... | 0 | 343 | 1 | 0 | 31.2 | 0.5
    gloopblocks: | 0 | 370 | 0 | 0 | 41.0 | 0.0
  • ABM Writes all players to disk, every minute #40 ............................................ | 0 | 370 | 0 | 0 | 41.0 | 0.0
    mushroom: | 0 | 595 | 0 | 0 | 50.2 | 0.0
  • ABM cppcheck warnings #4 ............................................. | 0 | 595 | 0 | 0 | 50.2 | 0.0
    clams: | 0 | 9 | 0 | 0 | 1.0 | 0.0
  • ABM cppcheck warnings #4 ............................................. | 0 | 9 | 0 | 0 | 0.6 | 0.0
  • ABM Furnace segfault #3 ............................................. | 0 | 8 | 0 | 0 | 1.0 | 0.0
    hunger: | 0 | 541 | 6 | 0 | 55.5 | 1.2
  • globalstep GlowStone code #1 ...................................... | 0 | 541 | 6 | 0 | 55.5 | 1.2
    3d_armor: | 0 | 651 | 4 | 0 | 33.7 | 0.7
  • globalstep GlowStone code #1 ...................................... | 0 | 651 | 4 | 0 | 33.7 | 0.7
    seaplants: | 0 | 29 | 0 | 0 | 2.3 | 0.0
  • ABM Fixed key names so the key set menu now works. #5 ............................................. | 0 | 13 | 0 | 0 | 0.5 | 0.0
  • ABM Burned wood #2 ............................................. | 0 | 8 | 0 | 0 | 0.6 | 0.0
  • ABM OK, Saplings For Real #9 ............................................. | 0 | 10 | 0 | 0 | 1.2 | 0.0
  • ABM GlowStone code #1 ............................................. | 0 | 24 | 0 | 0 | 1.9 | 0.0
  • ABM Saplings + Growing Trees #8 ............................................. | 0 | 29 | 0 | 0 | 2.3 | 0.0
  • ABM Fixed null pointer dereference errors found by cppcheck #10 ............................................ | 0 | 18 | 0 | 0 | 1.2 | 0.0
    plants_lib: | 2 | 348 | 5 | 0 | 46.2 | 1.4
  • globalstep GlowStone code #1 ...................................... | 1 | 289 | 3 | 0 | 24.9 | 0.7
  • globalstep Burned wood #2 ...................................... | 0 | 345 | 2 | 0 | 45.8 | 0.6
    mobs_animal: | 8 | 3264 | 36 | 0 | 67.5 | 7.4
  • ABM Furnace segfault #3 ............................................. | 0 | 1 | 0 | 0 | 0.1 | 0.0
  • ABM Burned wood #2 ............................................. | 0 | 1 | 0 | 0 | 0.1 | 0.0
  • [mobs_animal:bunny].on_step() ...................... | 8 | 3264 | 36 | 0 | 67.5 | 7.4
  • ABM GlowStone code #1 ............................................. | 0 | 1 | 0 | 0 | 0.2 | 0.0
  • ABM Saplings + Growing Trees #8 ............................................. | 0 | 1 | 0 | 0 | 0.1 | 0.0
  • ABM cppcheck warnings #4 ............................................. | 0 | 1 | 0 | 0 | 0.1 | 0.0
    firearmslib: | 0 | 2699 | 6 | 0 | 78.8 | 0.8
  • globalstep GlowStone code #1 ...................................... | 0 | 2699 | 6 | 0 | 78.8 | 0.8
    builtin: | 14 | 2394 | 48 | 0 | 100.0 | 9.9
  • globalstep GlowStone code #1 ...................................... | 3 | 643 | 9 | 0 | 68.2 | 2.3
  • on_chat_message GlowStone code #1 ................................. | 0 | 2365 | 2 | 0 | 100.0 | 0.1
  • [:__builtin:item].on_step() ........................ | 11 | 1856 | 36 | 0 | 78.8 | 7.5
    walking_light: | 0 | 7625 | 24 | 0 | 57.2 | 3.3
  • globalstep GlowStone code #1 ...................................... | 0 | 7625 | 24 | 0 | 57.2 | 3.3
    anti_grief: | 0 | 1060 | 4 | 0 | 65.2 | 0.8
  • globalstep GlowStone code #1 ...................................... | 0 | 1060 | 4 | 0 | 65.2 | 0.8
    moretrees: | 0 | 2220 | 14 | 0 | 54.2 | 1.2
  • ABM Dungeon Masters are immortal in fact #42 ............................................ | 0 | 2220 | 6 | 0 | 51.3 | 0.6
  • ABM Finnish translation #44 ............................................ | 0 | 893 | 5 | 0 | 39.8 | 0.3
  • ABM Install icon and .desktop file on Unix systems #48 ............................................ | 0 | 301 | 2 | 0 | 19.1 | 0.1
  • ABM . #54 ............................................ | 0 | 69 | 0 | 0 | 13.5 | 0.0
    mesecons: | 3 | 720 | 9 | 0 | 34.1 | 2.0
  • globalstep GlowStone code #1 ...................................... | 3 | 720 | 9 | 0 | 34.1 | 2.0
    playereffects: | 1 | 742 | 7 | 0 | 69.3 | 1.1
  • globalstep GlowStone code #1 ...................................... | 1 | 742 | 7 | 0 | 69.3 | 1.1
    default: | 8 | 11573 | 91 | 0 | 96.2 | 10.0
  • ABM Fix build with Pathscale's EKOPath #7 ............................................. | 0 | 11401 | 44 | 0 | 94.8 | 2.0
  • ABM Apples on the trees can not be eaten #6 ............................................. | 0 | 10125 | 13 | 0 | 77.6 | 1.0
  • globalstep GlowStone code #1 ...................................... | 1 | 102 | 1 | 0 | 14.0 | 0.5
  • globalstep Burned wood #2 ...................................... | 7 | 1198 | 27 | 0 | 80.3 | 6.2
  • ABM Fixed key names so the key set menu now works. #5 ............................................. | 0 | 1301 | 5 | 0 | 40.1 | 0.3
    ---------------------------------------------------------+-----------+-----------+-----------+-------+-------+-------
    total: | 136 | 27308 | 566 | 100 | 100.0 | 100.0

@t4im
Copy link
Contributor

t4im commented Jun 23, 2016

Alright, thanks :-)

Good idea to increase the precision of the relative values.

@JDCodeIt
Copy link
Contributor Author

Not sure why the OSX unit test failed on threading... do you need to
resubmit?

On 06/23/2016 03:43 AM, Tim wrote:

Alright, thanks :-)

Good idea to increase the precision of the relative values.


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#4220 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/ALoEtGfTzUBveSO3MZvo_9LmJjkAfCCVks5qOlVXgaJpZM4I1c0S.

@t4im
Copy link
Contributor

t4im commented Jun 30, 2016

That's apparently a known problem, which is not related to the changes in the PR.

@est31
Copy link
Contributor

est31 commented Jul 12, 2016

d7060c2

@est31 est31 closed this as completed Jul 12, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Issues that were confirmed to be a bug
Projects
None yet
Development

No branches or pull requests

4 participants