Arena is crufty?
Ken's PODRedBlackTree patch has made me go back and take a closer look at WebKit's Arena "class". Turns out it's not a class at all, just some structs and macros. That seems very un-WebKit-like to me. Ken's patch also has a PODArena class, which uses Arena in its implementation. Sam suggests that PODRedBlackTree should really go into WTF, which means PODArena and Arena would need to go there as well. It seems like Arena really needs to be brought into the 21st century and made a proper class. Maybe now is the right time to: 1) Make Arena a class 2) Integrate Ken's PODArena functionality into this new Arena class (or maybe just make Ken's PODArena the new Arena class). 3) Move the new Arena class to WTF 4) Put PODRedBlackTree in WTF It looks like RenderArena is currently the only client of Arena.h, so this change shouldn't be too hard. Of course, looking at RenderArena, it's a little odd, too. It is not renderer specific at all. It's just an Arena that recycles freed objects. Maybe we should move that functionality into the new Arena class. But RenderArena is used all over the place, so maybe that's going one step too far down this road? ----- ~Chris cmarrin@apple.com
On Wed, Sep 1, 2010 at 4:20 PM, Chris Marrin <cmarrin@apple.com> wrote:
Ken's PODRedBlackTree patch has made me go back and take a closer look at WebKit's Arena "class". Turns out it's not a class at all, just some structs and macros. That seems very un-WebKit-like to me. Ken's patch also has a PODArena class, which uses Arena in its implementation. Sam suggests that PODRedBlackTree should really go into WTF, which means PODArena and Arena would need to go there as well.
It seems like Arena really needs to be brought into the 21st century and made a proper class. Maybe now is the right time to:
1) Make Arena a class
2) Integrate Ken's PODArena functionality into this new Arena class (or maybe just make Ken's PODArena the new Arena class).
3) Move the new Arena class to WTF
4) Put PODRedBlackTree in WTF
It looks like RenderArena is currently the only client of Arena.h, so this change shouldn't be too hard. Of course, looking at RenderArena, it's a little odd, too. It is not renderer specific at all. It's just an Arena that recycles freed objects. Maybe we should move that functionality into the new Arena class. But RenderArena is used all over the place, so maybe that's going one step too far down this road?
I'm pretty sure we want to delete RenderArena (since the minuscule-to-negative performance gain is not worth the extra complexity) at some point anyway. If that is still the case, then maybe it makes more sense to have PODArena be the only arena class in WebKit. I'm not sure we have to move PODArena / PODRedBlackTree to WTF right away, but it can't hurt. My gut feeling is that historically most classes that think they want Arenas actually do not need the extra complexity and we shouldn't encourage more users of arena classes without some discussion. I actually prefer having PODArena be an implementation detail of PODRedBlackTree and not exposed as a general utility until we have a clear need for something else to use it. - James
----- ~Chris cmarrin@apple.com
_______________________________________________ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
On Wed, Sep 1, 2010 at 4:36 PM, James Robinson <jamesr@google.com> wrote:
On Wed, Sep 1, 2010 at 4:20 PM, Chris Marrin <cmarrin@apple.com> wrote:
Ken's PODRedBlackTree patch has made me go back and take a closer look at WebKit's Arena "class". Turns out it's not a class at all, just some structs and macros. That seems very un-WebKit-like to me. Ken's patch also has a PODArena class, which uses Arena in its implementation. Sam suggests that PODRedBlackTree should really go into WTF, which means PODArena and Arena would need to go there as well.
It seems like Arena really needs to be brought into the 21st century and made a proper class. Maybe now is the right time to:
1) Make Arena a class
2) Integrate Ken's PODArena functionality into this new Arena class (or maybe just make Ken's PODArena the new Arena class).
3) Move the new Arena class to WTF
4) Put PODRedBlackTree in WTF
It looks like RenderArena is currently the only client of Arena.h, so this change shouldn't be too hard. Of course, looking at RenderArena, it's a little odd, too. It is not renderer specific at all. It's just an Arena that recycles freed objects. Maybe we should move that functionality into the new Arena class. But RenderArena is used all over the place, so maybe that's going one step too far down this road?
I'm pretty sure we want to delete RenderArena (since the minuscule-to-negative performance gain is not worth the extra complexity) at some point anyway. If that is still the case, then maybe it makes more sense to have PODArena be the only arena class in WebKit. I'm not sure we have to move PODArena / PODRedBlackTree to WTF right away, but it can't hurt. My gut feeling is that historically most classes that think they want Arenas actually do not need the extra complexity and we shouldn't encourage more users of arena classes without some discussion. I actually prefer having PODArena be an implementation detail of PODRedBlackTree and not exposed as a general utility until we have a clear need for something else to use it.
There is more code coming, outside of the red/black tree, which uses PODArena to allocate (lots) of small temporary objects and then throw them all away at the end. I definitely do not want to have to add bookkeeping for these temporary objects, which would be required if the PODArena weren't available. PODArena is not designed to be a replacement for the ArenaPool functionality; it's just a consumer. I'd rather start to land the code in this place before doing major restructurings. -Ken
- James
----- ~Chris cmarrin@apple.com
_______________________________________________ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
_______________________________________________ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
On Sep 1, 2010, at 4:20 PM, Chris Marrin wrote:
Ken's PODRedBlackTree patch has made me go back and take a closer look at WebKit's Arena "class". Turns out it's not a class at all, just some structs and macros. That seems very un-WebKit-like to me. Ken's patch also has a PODArena class, which uses Arena in its implementation. Sam suggests that PODRedBlackTree should really go into WTF, which means PODArena and Arena would need to go there as well.
It seems like Arena really needs to be brought into the 21st century and made a proper class. Maybe now is the right time to:
1) Make Arena a class
2) Integrate Ken's PODArena functionality into this new Arena class (or maybe just make Ken's PODArena the new Arena class).
3) Move the new Arena class to WTF
4) Put PODRedBlackTree in WTF
It looks like RenderArena is currently the only client of Arena.h, so this change shouldn't be too hard. Of course, looking at RenderArena, it's a little odd, too. It is not renderer specific at all. It's just an Arena that recycles freed objects. Maybe we should move that functionality into the new Arena class. But RenderArena is used all over the place, so maybe that's going one step too far down this road?
Arena was imported from Mozilla and could certainly benefit from modernization. For the rendering use case though, it is essential to handle non-POD types correctly. Regards, Maciej
On Sep 1, 2010, at 4:39 PM, Maciej Stachowiak wrote:
On Sep 1, 2010, at 4:20 PM, Chris Marrin wrote:
Ken's PODRedBlackTree patch has made me go back and take a closer look at WebKit's Arena "class". Turns out it's not a class at all, just some structs and macros. That seems very un-WebKit-like to me. Ken's patch also has a PODArena class, which uses Arena in its implementation. Sam suggests that PODRedBlackTree should really go into WTF, which means PODArena and Arena would need to go there as well.
It seems like Arena really needs to be brought into the 21st century and made a proper class. Maybe now is the right time to:
1) Make Arena a class
2) Integrate Ken's PODArena functionality into this new Arena class (or maybe just make Ken's PODArena the new Arena class).
3) Move the new Arena class to WTF
4) Put PODRedBlackTree in WTF
It looks like RenderArena is currently the only client of Arena.h, so this change shouldn't be too hard. Of course, looking at RenderArena, it's a little odd, too. It is not renderer specific at all. It's just an Arena that recycles freed objects. Maybe we should move that functionality into the new Arena class. But RenderArena is used all over the place, so maybe that's going one step too far down this road?
Arena was imported from Mozilla and could certainly benefit from modernization. For the rendering use case though, it is essential to handle non-POD types correctly.
But RenderArena doesn't deal with types at all, does it? It just allocs and free's void*'s. It's not even a template class. So maybe we should have an Arena class which deals with void*'s and a PODArena template class which lets you type arena objects? RenderArena would use the Arena class. ----- ~Chris cmarrin@apple.com
On Wed, Sep 1, 2010 at 5:08 PM, Chris Marrin <cmarrin@apple.com> wrote:
On Sep 1, 2010, at 4:39 PM, Maciej Stachowiak wrote:
On Sep 1, 2010, at 4:20 PM, Chris Marrin wrote:
Ken's PODRedBlackTree patch has made me go back and take a closer look at WebKit's Arena "class". Turns out it's not a class at all, just some structs and macros. That seems very un-WebKit-like to me. Ken's patch also has a PODArena class, which uses Arena in its implementation. Sam suggests that PODRedBlackTree should really go into WTF, which means PODArena and Arena would need to go there as well.
It seems like Arena really needs to be brought into the 21st century and made a proper class. Maybe now is the right time to:
1) Make Arena a class
2) Integrate Ken's PODArena functionality into this new Arena class (or maybe just make Ken's PODArena the new Arena class).
3) Move the new Arena class to WTF
4) Put PODRedBlackTree in WTF
It looks like RenderArena is currently the only client of Arena.h, so this change shouldn't be too hard. Of course, looking at RenderArena, it's a little odd, too. It is not renderer specific at all. It's just an Arena that recycles freed objects. Maybe we should move that functionality into the new Arena class. But RenderArena is used all over the place, so maybe that's going one step too far down this road?
Arena was imported from Mozilla and could certainly benefit from modernization. For the rendering use case though, it is essential to handle non-POD types correctly.
But RenderArena doesn't deal with types at all, does it? It just allocs and free's void*'s. It's not even a template class. So maybe we should have an Arena class which deals with void*'s and a PODArena template class which lets you type arena objects? RenderArena would use the Arena class.
RenderArena is used by classes which want to override operator new and delete in order to be allocated in an arena. PODArena is designed to be non-intrusive with respect to the POD data types that are allocated in it; overloaded operator new and delete do not need to be provided. -Ken
On Sep 1, 2010, at 5:12 PM, Kenneth Russell wrote:
On Wed, Sep 1, 2010 at 5:08 PM, Chris Marrin <cmarrin@apple.com> wrote:
On Sep 1, 2010, at 4:39 PM, Maciej Stachowiak wrote:
On Sep 1, 2010, at 4:20 PM, Chris Marrin wrote:
Ken's PODRedBlackTree patch has made me go back and take a closer look at WebKit's Arena "class". Turns out it's not a class at all, just some structs and macros. That seems very un-WebKit-like to me. Ken's patch also has a PODArena class, which uses Arena in its implementation. Sam suggests that PODRedBlackTree should really go into WTF, which means PODArena and Arena would need to go there as well.
It seems like Arena really needs to be brought into the 21st century and made a proper class. Maybe now is the right time to:
1) Make Arena a class
2) Integrate Ken's PODArena functionality into this new Arena class (or maybe just make Ken's PODArena the new Arena class).
3) Move the new Arena class to WTF
4) Put PODRedBlackTree in WTF
It looks like RenderArena is currently the only client of Arena.h, so this change shouldn't be too hard. Of course, looking at RenderArena, it's a little odd, too. It is not renderer specific at all. It's just an Arena that recycles freed objects. Maybe we should move that functionality into the new Arena class. But RenderArena is used all over the place, so maybe that's going one step too far down this road?
Arena was imported from Mozilla and could certainly benefit from modernization. For the rendering use case though, it is essential to handle non-POD types correctly.
But RenderArena doesn't deal with types at all, does it? It just allocs and free's void*'s. It's not even a template class. So maybe we should have an Arena class which deals with void*'s and a PODArena template class which lets you type arena objects? RenderArena would use the Arena class.
RenderArena is used by classes which want to override operator new and delete in order to be allocated in an arena.
PODArena is designed to be non-intrusive with respect to the POD data types that are allocated in it; overloaded operator new and delete do not need to be provided.
Right, so I think we should have (in WTF) an Arena class which simply does arena based alloc and free of void*'s, and a PODArena template class which does what your PODArena does today, using the Arena class as its implementation. Then RenderArena should be changed to use Arena. This is minimal change and doesn't change the functionality of any end-user classes. ----- ~Chris cmarrin@apple.com
We should just kill Arena and remove it and RenderArena both. dave On Sep 1, 2010, at 4:20 PM, Chris Marrin wrote:
Ken's PODRedBlackTree patch has made me go back and take a closer look at WebKit's Arena "class". Turns out it's not a class at all, just some structs and macros. That seems very un-WebKit-like to me. Ken's patch also has a PODArena class, which uses Arena in its implementation. Sam suggests that PODRedBlackTree should really go into WTF, which means PODArena and Arena would need to go there as well.
It seems like Arena really needs to be brought into the 21st century and made a proper class. Maybe now is the right time to:
1) Make Arena a class
2) Integrate Ken's PODArena functionality into this new Arena class (or maybe just make Ken's PODArena the new Arena class).
3) Move the new Arena class to WTF
4) Put PODRedBlackTree in WTF
It looks like RenderArena is currently the only client of Arena.h, so this change shouldn't be too hard. Of course, looking at RenderArena, it's a little odd, too. It is not renderer specific at all. It's just an Arena that recycles freed objects. Maybe we should move that functionality into the new Arena class. But RenderArena is used all over the place, so maybe that's going one step too far down this road?
----- ~Chris cmarrin@apple.com
_______________________________________________ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
On Sep 1, 2010, at 7:04 PM, David Hyatt wrote:
We should just kill Arena and remove it and RenderArena both.
Wasn't it still a measurable slowdown last time we tried that? - Maciej
On Sep 1, 2010, at 4:20 PM, Chris Marrin wrote:
Ken's PODRedBlackTree patch has made me go back and take a closer look at WebKit's Arena "class". Turns out it's not a class at all, just some structs and macros. That seems very un-WebKit-like to me. Ken's patch also has a PODArena class, which uses Arena in its implementation. Sam suggests that PODRedBlackTree should really go into WTF, which means PODArena and Arena would need to go there as well.
It seems like Arena really needs to be brought into the 21st century and made a proper class. Maybe now is the right time to:
1) Make Arena a class
2) Integrate Ken's PODArena functionality into this new Arena class (or maybe just make Ken's PODArena the new Arena class).
3) Move the new Arena class to WTF
4) Put PODRedBlackTree in WTF
It looks like RenderArena is currently the only client of Arena.h, so this change shouldn't be too hard. Of course, looking at RenderArena, it's a little odd, too. It is not renderer specific at all. It's just an Arena that recycles freed objects. Maybe we should move that functionality into the new Arena class. But RenderArena is used all over the place, so maybe that's going one step too far down this road?
On Sep 1, 2010, at 7:07 PM, Maciej Stachowiak wrote:
On Sep 1, 2010, at 7:04 PM, David Hyatt wrote:
We should just kill Arena and remove it and RenderArena both.
Wasn't it still a measurable slowdown last time we tried that?
Not enough to matter. We could easily make up the difference. dave (hyatt@apple.com)
Please let's not add another client of Arena though. That will just make it harder to remove, and I highly doubt you're getting any real performance gain from using it. dave
I would be happy to not add another Arena client, but the primary reason I need an arena is not just for performance but to avoid having to keep track of all of the objects I need to delete. Is there any consensus yet on how to proceed with https://bugs.webkit.org/show_bug.cgi?id=45059 ? I'm concerned about taking on large-scale restructuring with potential performance impact as a prerequisite for my landing any initial code. I could revert my PODArena class to use its own memory allocation rather than that in Arena.h. -Ken On Wed, Sep 1, 2010 at 7:12 PM, David Hyatt <hyatt@apple.com> wrote:
Please let's not add another client of Arena though. That will just make it harder to remove, and I highly doubt you're getting any real performance gain from using it.
dave
_______________________________________________ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Isn't ref counting supposed to be _really_ optimized for exactly this use? It seems like a good match--unless you have major issues with cycles...which might be the issue? J On Thu, Sep 2, 2010 at 3:20 AM, Kenneth Russell <kbr@google.com> wrote:
I would be happy to not add another Arena client, but the primary reason I need an arena is not just for performance but to avoid having to keep track of all of the objects I need to delete.
Is there any consensus yet on how to proceed with https://bugs.webkit.org/show_bug.cgi?id=45059 ? I'm concerned about taking on large-scale restructuring with potential performance impact as a prerequisite for my landing any initial code. I could revert my PODArena class to use its own memory allocation rather than that in Arena.h.
-Ken
On Wed, Sep 1, 2010 at 7:12 PM, David Hyatt <hyatt@apple.com> wrote:
Please let's not add another client of Arena though. That will just make it harder to remove, and I highly doubt you're getting any real performance gain from using it.
dave
_______________________________________________ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
_______________________________________________ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Reference counting is not a solution. It still requires pointers to track objects. KBR is avoiding the overhead of storing pointers to track objects. That's the point of using an arena allocator for things that do not have destructors. -Darin On Thu, Sep 2, 2010 at 3:15 AM, Jeremy Orlow <jorlow@chromium.org> wrote:
Isn't ref counting supposed to be _really_ optimized for exactly this use? It seems like a good match--unless you have major issues with cycles...which might be the issue?
J
On Thu, Sep 2, 2010 at 3:20 AM, Kenneth Russell <kbr@google.com> wrote:
I would be happy to not add another Arena client, but the primary reason I need an arena is not just for performance but to avoid having to keep track of all of the objects I need to delete.
Is there any consensus yet on how to proceed with https://bugs.webkit.org/show_bug.cgi?id=45059 ? I'm concerned about taking on large-scale restructuring with potential performance impact as a prerequisite for my landing any initial code. I could revert my PODArena class to use its own memory allocation rather than that in Arena.h.
-Ken
On Wed, Sep 1, 2010 at 7:12 PM, David Hyatt <hyatt@apple.com> wrote:
Please let's not add another client of Arena though. That will just make it harder to remove, and I highly doubt you're getting any real performance gain from using it.
dave
_______________________________________________ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
_______________________________________________ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
_______________________________________________ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
There are two cyclic data structures in the code I am adding. I would not want to have to retrofit them with reference counting, at least not before an initial commit so that there is a checkpoint. -Ken On Thu, Sep 2, 2010 at 3:15 AM, Jeremy Orlow <jorlow@chromium.org> wrote:
Isn't ref counting supposed to be _really_ optimized for exactly this use? It seems like a good match--unless you have major issues with cycles...which might be the issue? J
On Thu, Sep 2, 2010 at 3:20 AM, Kenneth Russell <kbr@google.com> wrote:
I would be happy to not add another Arena client, but the primary reason I need an arena is not just for performance but to avoid having to keep track of all of the objects I need to delete.
Is there any consensus yet on how to proceed with https://bugs.webkit.org/show_bug.cgi?id=45059 ? I'm concerned about taking on large-scale restructuring with potential performance impact as a prerequisite for my landing any initial code. I could revert my PODArena class to use its own memory allocation rather than that in Arena.h.
-Ken
On Wed, Sep 1, 2010 at 7:12 PM, David Hyatt <hyatt@apple.com> wrote:
Please let's not add another client of Arena though. That will just make it harder to remove, and I highly doubt you're getting any real performance gain from using it.
dave
_______________________________________________ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
_______________________________________________ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
On Sep 1, 2010, at 7:20 PM, Kenneth Russell wrote:
I would be happy to not add another Arena client, but the primary reason I need an arena is not just for performance but to avoid having to keep track of all of the objects I need to delete.
Is there any consensus yet on how to proceed with https://bugs.webkit.org/show_bug.cgi?id=45059 ? I'm concerned about taking on large-scale restructuring with potential performance impact as a prerequisite for my landing any initial code. I could revert my PODArena class to use its own memory allocation rather than that in Arena.h.
I just posted that it seems like your RB tree could be replaced by std::multimap. And, given comments from others, it seems like the right thing to do with Arena is to put PODArena into the gpu directory like you were originally going to do, but to not use Arena.h (suck it's functionality into PODArena). Alternately, you could try Jeremy's idea and ref count your objects. If you use std::multimap, elements can be of type RefPtr<something>, so you can avoid all memory management issues. ----- ~Chris cmarrin@apple.com
On Thu, Sep 2, 2010 at 8:51 AM, Chris Marrin <cmarrin@apple.com> wrote:
On Sep 1, 2010, at 7:20 PM, Kenneth Russell wrote:
I would be happy to not add another Arena client, but the primary reason I need an arena is not just for performance but to avoid having to keep track of all of the objects I need to delete.
Is there any consensus yet on how to proceed with https://bugs.webkit.org/show_bug.cgi?id=45059 ? I'm concerned about taking on large-scale restructuring with potential performance impact as a prerequisite for my landing any initial code. I could revert my PODArena class to use its own memory allocation rather than that in Arena.h.
I just posted that it seems like your RB tree could be replaced by std::multimap. And, given comments from others, it seems like the right thing to do with Arena is to put PODArena into the gpu directory like you were originally going to do, but to not use Arena.h (suck it's functionality into PODArena). Alternately, you could try Jeremy's idea and ref count your objects. If you use std::multimap, elements can be of type RefPtr<something>, so you can avoid all memory management issues.
I thought there was a policy against using STL container classes in WebKit. Am I imagining things? Aren't there concerns about how STL container classes handle OOM situations, throwing exceptions, and so on? If the conclusion is for PODArena to live in graphics/gpu/ as originally proposed (without a dependency on Arena.h as originally written), and if using STL container classes is indeed verboten, then I'd like to propose that PODRedBlackTree also have its home be in graphics/gpu/. If there comes a time that we want to use these classes elsewhere in WebKit, then we can move them to a more general location, but at least they will be built as generic classes from the start. I can't tell who needs to agree to this before we can move forward. Any objections? Regards, -Darin
On Thu, Sep 2, 2010 at 8:51 AM, Chris Marrin <cmarrin@apple.com> wrote:
On Sep 1, 2010, at 7:20 PM, Kenneth Russell wrote:
I would be happy to not add another Arena client, but the primary reason I need an arena is not just for performance but to avoid having to keep track of all of the objects I need to delete.
Is there any consensus yet on how to proceed with https://bugs.webkit.org/show_bug.cgi?id=45059 ? I'm concerned about taking on large-scale restructuring with potential performance impact as a prerequisite for my landing any initial code. I could revert my PODArena class to use its own memory allocation rather than that in Arena.h.
I just posted that it seems like your RB tree could be replaced by std::multimap. And, given comments from others, it seems like the right thing to do with Arena is to put PODArena into the gpu directory like you were originally going to do, but to not use Arena.h (suck it's functionality into PODArena). Alternately, you could try Jeremy's idea and ref count your objects. If you use std::multimap, elements can be of type RefPtr<something>, so you can avoid all memory management issues.
I haven't seen that reply yet, but replacing my red-black tree with std::multimap is not a solution. My red-black tree is specifically designed to be augmentable, and the IntervalTree built on it is a core data structure used in the path processing code. -Ken
On Sep 2, 2010, at 9:41 AM, Kenneth Russell wrote:
On Thu, Sep 2, 2010 at 8:51 AM, Chris Marrin <cmarrin@apple.com> wrote:
On Sep 1, 2010, at 7:20 PM, Kenneth Russell wrote:
I would be happy to not add another Arena client, but the primary reason I need an arena is not just for performance but to avoid having to keep track of all of the objects I need to delete.
Is there any consensus yet on how to proceed with https://bugs.webkit.org/show_bug.cgi?id=45059 ? I'm concerned about taking on large-scale restructuring with potential performance impact as a prerequisite for my landing any initial code. I could revert my PODArena class to use its own memory allocation rather than that in Arena.h.
I just posted that it seems like your RB tree could be replaced by std::multimap. And, given comments from others, it seems like the right thing to do with Arena is to put PODArena into the gpu directory like you were originally going to do, but to not use Arena.h (suck it's functionality into PODArena). Alternately, you could try Jeremy's idea and ref count your objects. If you use std::multimap, elements can be of type RefPtr<something>, so you can avoid all memory management issues.
I haven't seen that reply yet, but replacing my red-black tree with std::multimap is not a solution. My red-black tree is specifically designed to be augmentable, and the IntervalTree built on it is a core data structure used in the path processing code.
The wheels go round and round. Seems like the right solution is to put PODRedBlackTree and PODArena in gpu as originally planned. But still suck in the functionality of Arena.h rather than using it directly. That gives us the option of getting rid of Arena.h at some point. ----- ~Chris cmarrin@apple.com
On Thu, Sep 2, 2010 at 9:59 AM, Chris Marrin <cmarrin@apple.com> wrote:
On Sep 2, 2010, at 9:41 AM, Kenneth Russell wrote:
On Thu, Sep 2, 2010 at 8:51 AM, Chris Marrin <cmarrin@apple.com> wrote:
On Sep 1, 2010, at 7:20 PM, Kenneth Russell wrote:
I would be happy to not add another Arena client, but the primary reason I need an arena is not just for performance but to avoid having to keep track of all of the objects I need to delete.
Is there any consensus yet on how to proceed with https://bugs.webkit.org/show_bug.cgi?id=45059 ? I'm concerned about taking on large-scale restructuring with potential performance impact as a prerequisite for my landing any initial code. I could revert my PODArena class to use its own memory allocation rather than that in Arena.h.
I just posted that it seems like your RB tree could be replaced by std::multimap. And, given comments from others, it seems like the right thing to do with Arena is to put PODArena into the gpu directory like you were originally going to do, but to not use Arena.h (suck it's functionality into PODArena). Alternately, you could try Jeremy's idea and ref count your objects. If you use std::multimap, elements can be of type RefPtr<something>, so you can avoid all memory management issues.
I haven't seen that reply yet, but replacing my red-black tree with std::multimap is not a solution. My red-black tree is specifically designed to be augmentable, and the IntervalTree built on it is a core data structure used in the path processing code.
The wheels go round and round.
Seems like the right solution is to put PODRedBlackTree and PODArena in gpu as originally planned. But still suck in the functionality of Arena.h rather than using it directly. That gives us the option of getting rid of Arena.h at some point.
Done. New patch coming soon. No take backs. -Ken
On Sep 2, 2010, at 8:51 AM, Chris Marrin wrote:
On Sep 1, 2010, at 7:20 PM, Kenneth Russell wrote:
I would be happy to not add another Arena client, but the primary reason I need an arena is not just for performance but to avoid having to keep track of all of the objects I need to delete.
Is there any consensus yet on how to proceed with https://bugs.webkit.org/show_bug.cgi?id=45059 ? I'm concerned about taking on large-scale restructuring with potential performance impact as a prerequisite for my landing any initial code. I could revert my PODArena class to use its own memory allocation rather than that in Arena.h.
I just posted that it seems like your RB tree could be replaced by std::multimap. And, given comments from others, it seems like the right thing to do with Arena is to put PODArena into the gpu directory like you were originally going to do, but to not use Arena.h (suck it's functionality into PODArena). Alternately, you could try Jeremy's idea and ref count your objects. If you use std::multimap, elements can be of type RefPtr<something>, so you can avoid all memory management issues.
We usually don't use STL containers because they can raise exceptions and it's WebKit policy to not use exceptions in code, and to support building with exceptions disabled. Long ago, WebCore had a red-black tree class written by me but we managed to replace all uses with hashtables. I am assuming in this case a balanced tree is genuinely needed. If that is the case, I'd much prefer to see one that supports more than just POD types; a limitation to POD types is a significant loss of generality. It is not hard to make a balanced tree class that supports arbitrary C++ types, in fact it would be the natural way to code it. If the arena exists to avoid memory management bookkeeping and not for performance reasons, then I'm not sure that is a very good reason to have it. If it is required for performance, then it is reasonable to make it an implementation detail of the RB tree. Regards, Maciej
participants (7)
-
Chris Marrin
-
Darin Fisher
-
David Hyatt
-
James Robinson
-
Jeremy Orlow
-
Kenneth Russell
-
Maciej Stachowiak