Skip to content
This repository was archived by the owner on May 21, 2022. It is now read-only.

Feature:obj verify#6

Merged
pomma89 merged 3 commits intopomma89:masterfrom
uliian:feature-objVerify
Jul 1, 2017
Merged

Feature:obj verify#6
pomma89 merged 3 commits intopomma89:masterfrom
uliian:feature-objVerify

Conversation

@uliian
Copy link
Copy Markdown

@uliian uliian commented Jun 30, 2017

add base on timer evictor,tmed validation pooled objects
Add System.Threading.Timer package refrence,support Timer for .NET Standard 1.0

@pomma89 pomma89 merged commit 479a9f1 into pomma89:master Jul 1, 2017
@pomma89
Copy link
Copy Markdown
Owner

pomma89 commented Jul 1, 2017

Hi @uliian,

Thanks for your PR. I merged it just now, but, before releasing it, I would like to tweak it a bit; I'll try to do that today and in the following days.

All in all, it seems a very interesting PR, thank you!

As for PR #4, I will let you know which changes I did and why.

@pomma89
Copy link
Copy Markdown
Owner

pomma89 commented Jul 2, 2017

Hi @uliian,

You can find on the master branch all changes I made to your PR. This is, more or less, the list of all changes I made:

  • Timed object pool now uses your eviction system, that's really good.
  • Some minor aesthetic changes, like renaming.
  • The logic inside StartEvictor seemed flawed, because when an object was validated, it did not return to the pool. Therefore, I took the same logic which was in TimedObjectPool and used it instead.
  • The additional states (Validating, ValidateSuccess, ValidateFailure) were not needed. The idea is to keep those states to the minimum.
  • I removed the BorrowCount property. The reasoning is this: there are really many interesting properties which could be tracked, like when an object is created, for how long it is used, and so on; however, for performance reasons, we cannot keep track of them all. Thus, instead of choosing a few properties to keep track of, I think it's better to not track any property at all. Anyway, please have a look at how the TimedObjectPool class overloads CreatePooledObject to store additional info in the Payload property.

Let me know if, all in all, what is in the master branch is still good for you.

Thanks again for your PR.

@uliian
Copy link
Copy Markdown
Author

uliian commented Jul 3, 2017

Admire your sense of responsibility, I agree with you.

@pomma89
Copy link
Copy Markdown
Owner

pomma89 commented Jul 3, 2017

Hi @uliian,

The new package should be available on NuGet. Thanks again.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants