Giter VIP home page Giter VIP logo

Comments (6)

jtmueller avatar jtmueller commented on June 29, 2024 1

Renaming the method to something other that Dispose is a non-starter, because the whole point of this is to be able to use the object with a using statement, and have the underlying array automatically returned to the pool at the end of that block of code. The IDisposable interface requires the name Dispose, and explicit interface implementation plus renaming the public Dispose method would break the published API of these objects and add confusion for people using them.

I could set a boolean flag when you call Dispose, and then check that flag in each and every method and property so I can throw an ObjectDisposedException if you try to use the object after disposing it. I just don't see the point of it. That's a whole lot of tedious work that also inserts at least two IL operations into each method and property, for no tangible benefit.

Anyone who is familiar with the IDisposable pattern is aware that you shouldn't count on being able to use an object after disposing it. Just because technically you can use the object after disposing it in this case doesn't change the general rule. Where is the actual harm done by the fact that doing something you're not supposed to in the general case doesn't actually break things in this case?

So, we can hurt performance of operations in tight loops by checking a class-level variable on every operation, and in exchange we get.. I'm not really sure. I'm afraid I don't see a compelling reason to make any change in this area.

from collections.pooled.

jtmueller avatar jtmueller commented on June 29, 2024 1

Okay, so I will grant that the fact that a PooledList is technically still usable after calling Dispose is an implementation detail that would not be obvious to someone who had not read the source code.

In your scenario, someone who assumes that they can't re-use a list after calling Dispose still has options, though. That person could call list.Clear();, followed by list.Capacity = 0;. This is basically the equivalent of calling Dispose, and will return the rented array to the the array pool. In fact, you wouldn't even need to call Clear because just setting the capacity to zero will take care of returning the array to the pool and settinglist.Count to zero.

Since you can get the same memory characteristics as in your Dispose scenario by using the Capacity property, even if you don't know pooled lists are still usable after Dispose, would it be acceptable to leave Dispose as-is, but add some additional documentation to the Capacity property to let the user know what happens when it's set to zero?

from collections.pooled.

dzmitry-lahoda avatar dzmitry-lahoda commented on June 29, 2024

Use case is next.

The is hundred pooled lists. There is a timer which beats 60 times per second. Each time, code sparsely and randomly initializes some lists with one to thousand elements. When list is used, it can be Clear. But clear still retains memory from pool. So total memory used and memory per list (current rented array) is bigger than if to call Dispose directly each time. Knowing that after Dispose list could be reused. So dispose somewhat changes meaning...

from collections.pooled.

dzmitry-lahoda avatar dzmitry-lahoda commented on June 29, 2024
ArgumentOutOfRangeException
Capacity is set to a value that is less than Count.

I guess better to retain semantics of Capacity setter of List(no checked code of PooledList if semantics already changed now). Dispose may be retained to possible usage when list cannot be used after dispose, if ever case appears, if. Other options to have Count setter ('List` has only getter) which returns to pool, but unclear if case of partial return is real.

So, I will check Clear (in case of ctor parameter do not clear data on free, clear should not clear either) and Capacity = 0 if it works as good as Dispose(same code executed). And name it in extensions method like ReturnArray or like (seems nor Return nor Free should not be used as is).

from collections.pooled.

jtmueller avatar jtmueller commented on June 29, 2024

Ah, good point. Capacity by itself wouldn't work, but Clear, then Capacity should do the trick.

from collections.pooled.

dzmitry-lahoda avatar dzmitry-lahoda commented on June 29, 2024

I have extension ClearCapacity() which works.

from collections.pooled.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.