Implement prototype of flexible hook handler class #74

Merged
merged 11 commits into from Sep 18, 2016

Conversation

Projects
None yet
2 participants
@syucream
Owner

syucream commented Sep 7, 2016

#36

  • Separate logic related to thread shared/local data.
  • Add a new class, load handler and insert appropriate hook plugins.
  • Refactoring.

- [ ] Modify unit test code will be fixed by #75

@coveralls

This comment has been minimized.

Show comment
Hide comment
@coveralls

coveralls Sep 7, 2016

Coverage Status

Coverage decreased (-51.7%) to 0.0% when pulling bf60a27 on feature/hook_prototype into f4b525a on master.

Coverage Status

Coverage decreased (-51.7%) to 0.0% when pulling bf60a27 on feature/hook_prototype into f4b525a on master.

@syucream

This comment has been minimized.

Show comment
Hide comment
@syucream

syucream Sep 7, 2016

Owner

Idea notes

Requirements

  • low memory consumption
  • safely memory access

Idea

  • Enforce users to create their handler class(or module?). For example:
module MyEventHandler
  def on_send_request_hdr
    ...
  end
end
  • Generate new obj of the class from current thread-local mrb_state.
    • Avoid to create new mrb_state.
    • Is it unsafely access?
  • To avoid unexpected GC, I must mark the new obj.
    • Stop GC temporarily by calling mrb_gc_register()
    • And restart GC by calling mrb_gc_unregister()
    • It has a risk of missing mrb_gc_unregister() so I should take RAII approach, for example using shared_ptr.
  • Insert a new TransactionPlugin registers hooks and has hook handler functions.
    • Execute the obj by calling mrb_funcall_*()
Owner

syucream commented Sep 7, 2016

Idea notes

Requirements

  • low memory consumption
  • safely memory access

Idea

  • Enforce users to create their handler class(or module?). For example:
module MyEventHandler
  def on_send_request_hdr
    ...
  end
end
  • Generate new obj of the class from current thread-local mrb_state.
    • Avoid to create new mrb_state.
    • Is it unsafely access?
  • To avoid unexpected GC, I must mark the new obj.
    • Stop GC temporarily by calling mrb_gc_register()
    • And restart GC by calling mrb_gc_unregister()
    • It has a risk of missing mrb_gc_unregister() so I should take RAII approach, for example using shared_ptr.
  • Insert a new TransactionPlugin registers hooks and has hook handler functions.
    • Execute the obj by calling mrb_funcall_*()
@syucream

This comment has been minimized.

Show comment
Hide comment
@syucream

syucream Sep 8, 2016

Owner

progress

Tried to fix #74 (comment) by c753613.

new issue

  • Detect unusable class/method usage.
    • For example, ATS::echo is unavailable at late hook.
    • Should these calling raise an exception?
Owner

syucream commented Sep 8, 2016

progress

Tried to fix #74 (comment) by c753613.

new issue

  • Detect unusable class/method usage.
    • For example, ATS::echo is unavailable at late hook.
    • Should these calling raise an exception?
@coveralls

This comment has been minimized.

Show comment
Hide comment
@coveralls

coveralls Sep 8, 2016

Coverage Status

Coverage decreased (-51.7%) to 0.0% when pulling 98dacd8 on feature/hook_prototype into f4b525a on master.

Coverage Status

Coverage decreased (-51.7%) to 0.0% when pulling 98dacd8 on feature/hook_prototype into f4b525a on master.

@coveralls

This comment has been minimized.

Show comment
Hide comment
@coveralls

coveralls Sep 8, 2016

Coverage Status

Coverage decreased (-51.7%) to 0.0% when pulling 98dacd8 on feature/hook_prototype into f4b525a on master.

Coverage Status

Coverage decreased (-51.7%) to 0.0% when pulling 98dacd8 on feature/hook_prototype into f4b525a on master.

@syucream

This comment has been minimized.

Show comment
Hide comment
@syucream

syucream Sep 10, 2016

Owner

To detect unusable class/method usage, plugin must pass hook timing to the transaction because atscppapi::Transaction doesn't have hook info API.

Owner

syucream commented Sep 10, 2016

To detect unusable class/method usage, plugin must pass hook timing to the transaction because atscppapi::Transaction doesn't have hook info API.

@coveralls

This comment has been minimized.

Show comment
Hide comment
@coveralls

coveralls Sep 11, 2016

Coverage Status

Coverage decreased (-51.7%) to 0.0% when pulling 42bf9bb on feature/hook_prototype into f4b525a on master.

Coverage Status

Coverage decreased (-51.7%) to 0.0% when pulling 42bf9bb on feature/hook_prototype into f4b525a on master.

@coveralls

This comment has been minimized.

Show comment
Hide comment
@coveralls

coveralls Sep 12, 2016

Coverage Status

Coverage decreased (-51.7%) to 0.0% when pulling 54f187d on feature/hook_prototype into f4b525a on master.

Coverage Status

Coverage decreased (-51.7%) to 0.0% when pulling 54f187d on feature/hook_prototype into f4b525a on master.

@syucream

This comment has been minimized.

Show comment
Hide comment
@syucream

syucream Sep 14, 2016

Owner

TSMrubyValue implemented at c753613 has non-thread-safe function.
At a5dd106, I try implementing an another approach.

Owner

syucream commented Sep 14, 2016

TSMrubyValue implemented at c753613 has non-thread-safe function.
At a5dd106, I try implementing an another approach.

@coveralls

This comment has been minimized.

Show comment
Hide comment
@coveralls

coveralls Sep 14, 2016

Coverage Status

Coverage decreased (-51.7%) to 0.0% when pulling a5dd106 on feature/hook_prototype into f4b525a on master.

Coverage Status

Coverage decreased (-51.7%) to 0.0% when pulling a5dd106 on feature/hook_prototype into f4b525a on master.

@coveralls

This comment has been minimized.

Show comment
Hide comment
@coveralls

coveralls Sep 17, 2016

Coverage Status

Coverage decreased (-51.7%) to 0.0% when pulling 5ba68e8 on feature/hook_prototype into f4b525a on master.

Coverage Status

Coverage decreased (-51.7%) to 0.0% when pulling 5ba68e8 on feature/hook_prototype into f4b525a on master.

@syucream

This comment has been minimized.

Show comment
Hide comment
@syucream

syucream Sep 17, 2016

Owner

Modify unit test code

Its a big task. I fix it as another issue.
#75

Owner

syucream commented Sep 17, 2016

Modify unit test code

Its a big task. I fix it as another issue.
#75

@coveralls

This comment has been minimized.

Show comment
Hide comment
@coveralls

coveralls Sep 17, 2016

Coverage Status

Coverage decreased (-51.7%) to 0.0% when pulling 706c841 on feature/hook_prototype into f4b525a on master.

Coverage Status

Coverage decreased (-51.7%) to 0.0% when pulling 706c841 on feature/hook_prototype into f4b525a on master.

@syucream syucream changed the title from [WIP] Implement prototype of flexible hook handler class to Implement prototype of flexible hook handler class Sep 18, 2016

@coveralls

This comment has been minimized.

Show comment
Hide comment
@coveralls

coveralls Sep 18, 2016

Coverage Status

Coverage decreased (-51.7%) to 0.0% when pulling a999df4 on feature/hook_prototype into f4b525a on master.

Coverage Status

Coverage decreased (-51.7%) to 0.0% when pulling a999df4 on feature/hook_prototype into f4b525a on master.

@syucream syucream merged commit 1078d3d into master Sep 18, 2016

3 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
coverage/coveralls Coverage remained the same at 0.0%
Details

@syucream syucream deleted the feature/hook_prototype branch Sep 18, 2016

@syucream syucream referenced this pull request Sep 21, 2016

Closed

redesign around hooks #36

3 of 3 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment