[webkit-dev] Checking allocation failures [was: Re: Naked new considered harmful]
Am 24.08.2010 19:46, schrieb Adam Barth:
One thing Darin and I discussed at WWDC (yes, this email has been a
long time coming) is better programming patterns to prevent memory
leaks. As I'm sure you know, whenever you allocate a RefCounted
object, you need to call adoptRef to prevent a memory leak by adopting
the object's initial reference:
adoptRef(new FancyRefCountedObject())
We now have an ASSERT that enforces this programming pattern, so we
should be able to catch errors pretty easily. Recently, Darin also
introduced an analogous function for OwnPtr:
adoptPtr(new NiftyNonRefCountedObject())
What adoptPtr does is shove the newly allocated object into a
PassOwnPtr, which together with OwnPtr, makes sure we don't leak the
object. By using one of these two functions every time we call new,
it's easy to see that we don't have any memory leaks. In the cases
where we have an intentional memory leak (e.g., for a static), please
use the leakPtr() member function to document the leak.
In writing new code, please avoid using naked calls to new. If
you're making an object that you expect to be heap-allocated, please
add a create method, similar to the create method we use for
RefCounted objects:
static PassOwnPtrNiftyNonRefCountedObject create(ParamClass* param)
{
return adoptPtr(new NiftyNonRefCountedObject(param));
}
You should also make the constructor non-public so folks are forced to
call the create method. (Of course, stack-allocated objects should
have public constructors.)
I'm going through WebCore and removing as many naked news and I can.
At some point, we'll add a stylebot rule to flag violations for new
code. If you'd like to help out, pick a directory and change all the
calls to new to use adoptRef or adoptPtr, as appropriate.
this reminds me that I've always been wondering about checks for
allocation failure in WebCore (versus concerns about leaks). A plain
call to new may throw an std::bad_alloc exception. If this is not
considered, it may leave objects in an invalid state, even when using
objects such as RefPtr to wrap arround allocations. I don't remember any
specific place in the WebCore code where it would be a problem, I just
don't remember seeing any code that deals with allocation failures. Is
this a design choice somehow? If so, is there some online
documentation/discussion about it? (Tried to google it but found nothing...)
Best regards,
-Stephan
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Checking allocation failures [was: Re: Naked new considered harmful]
Am 25.08.2010 18:35, schrieb Adam Barth:
On Wed, Aug 25, 2010 at 7:09 AM, Stephan Assmussupersti...@gmx.de wrote:
Am 24.08.2010 19:46, schrieb Adam Barth:
One thing Darin and I discussed at WWDC (yes, this email has been a
long time coming) is better programming patterns to prevent memory
leaks. As I'm sure you know, whenever you allocate a RefCounted
object, you need to call adoptRef to prevent a memory leak by adopting
the object's initial reference:
adoptRef(new FancyRefCountedObject())
We now have an ASSERT that enforces this programming pattern, so we
should be able to catch errors pretty easily. Recently, Darin also
introduced an analogous function for OwnPtr:
adoptPtr(new NiftyNonRefCountedObject())
What adoptPtr does is shove the newly allocated object into a
PassOwnPtr, which together with OwnPtr, makes sure we don't leak the
object. By using one of these two functions every time we call new,
it's easy to see that we don't have any memory leaks. In the cases
where we have an intentional memory leak (e.g., for a static), please
use the leakPtr() member function to document the leak.
In writing new code, please avoid using naked calls to new. If
you're making an object that you expect to be heap-allocated, please
add a create method, similar to the create method we use for
RefCounted objects:
static PassOwnPtrNiftyNonRefCountedObjectcreate(ParamClass* param)
{
return adoptPtr(new NiftyNonRefCountedObject(param));
}
You should also make the constructor non-public so folks are forced to
call the create method. (Of course, stack-allocated objects should
have public constructors.)
I'm going through WebCore and removing as many naked news and I can.
At some point, we'll add a stylebot rule to flag violations for new
code. If you'd like to help out, pick a directory and change all the
calls to new to use adoptRef or adoptPtr, as appropriate.
this reminds me that I've always been wondering about checks for allocation
failure in WebCore (versus concerns about leaks). A plain call to new may
throw an std::bad_alloc exception. If this is not considered, it may leave
objects in an invalid state, even when using objects such as RefPtr to wrap
arround allocations. I don't remember any specific place in the WebCore code
where it would be a problem, I just don't remember seeing any code that
deals with allocation failures. Is this a design choice somehow? If so, is
there some online documentation/discussion about it? (Tried to google it but
found nothing...)
Failed allocations in WebKit call CRASH(). This prevents us from
ending up in an inconsistent state.
Ok, but WebKit is used on devices where allocation failure is somewhat
of a realistic possibility, no? Wouldn't it be desirable to handle such
a situation gracefully? (I.e. display of an error message when trying to
open one more tab rather than crashing the entire browser, for example.)
Hopefully I am not missing something obvious.
Best regards,
-Stephan
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Checking allocation failures [was: Re: Naked new considered harmful]
Am 25.08.2010 23:34, schrieb Adam Barth:
On Wed, Aug 25, 2010 at 2:31 PM, Stephan Assmussupersti...@gmx.de wrote:
Am 25.08.2010 18:35, schrieb Adam Barth:
On Wed, Aug 25, 2010 at 7:09 AM, Stephan Assmussupersti...@gmx.de
wrote:
Am 24.08.2010 19:46, schrieb Adam Barth:
One thing Darin and I discussed at WWDC (yes, this email has been a
long time coming) is better programming patterns to prevent memory
leaks. As I'm sure you know, whenever you allocate a RefCounted
object, you need to call adoptRef to prevent a memory leak by adopting
the object's initial reference:
adoptRef(new FancyRefCountedObject())
We now have an ASSERT that enforces this programming pattern, so we
should be able to catch errors pretty easily. Recently, Darin also
introduced an analogous function for OwnPtr:
adoptPtr(new NiftyNonRefCountedObject())
What adoptPtr does is shove the newly allocated object into a
PassOwnPtr, which together with OwnPtr, makes sure we don't leak the
object. By using one of these two functions every time we call new,
it's easy to see that we don't have any memory leaks. In the cases
where we have an intentional memory leak (e.g., for a static), please
use the leakPtr() member function to document the leak.
In writing new code, please avoid using naked calls to new. If
you're making an object that you expect to be heap-allocated, please
add a create method, similar to the create method we use for
RefCounted objects:
static PassOwnPtrNiftyNonRefCountedObject create(ParamClass* param)
{
return adoptPtr(new NiftyNonRefCountedObject(param));
}
You should also make the constructor non-public so folks are forced to
call the create method. (Of course, stack-allocated objects should
have public constructors.)
I'm going through WebCore and removing as many naked news and I can.
At some point, we'll add a stylebot rule to flag violations for new
code. If you'd like to help out, pick a directory and change all the
calls to new to use adoptRef or adoptPtr, as appropriate.
this reminds me that I've always been wondering about checks for
allocation
failure in WebCore (versus concerns about leaks). A plain call to new may
throw an std::bad_alloc exception. If this is not considered, it may
leave
objects in an invalid state, even when using objects such as RefPtr to
wrap
arround allocations. I don't remember any specific place in the WebCore
code
where it would be a problem, I just don't remember seeing any code that
deals with allocation failures. Is this a design choice somehow? If so,
is
there some online documentation/discussion about it? (Tried to google it
but
found nothing...)
Failed allocations in WebKit call CRASH(). This prevents us from
ending up in an inconsistent state.
Ok, but WebKit is used on devices where allocation failure is somewhat of a
realistic possibility, no? Wouldn't it be desirable to handle such a
situation gracefully? (I.e. display of an error message when trying to open
one more tab rather than crashing the entire browser, for example.)
Hopefully I am not missing something obvious.
Dunno. The crash-on-allocation failure assumption is baked into lots
of lines of code.
I just thought that if my observations are correct, and on the subject
of advertising a certain way to write code (with regards to your initial
email), perhaps new code (and eventually old code) should also follow a
guideline that allows to handle allocation failures gracefully. For
example, if no allocations are to be done in constructors, but rather
within a dedicated init() method, objects remain always valid, even if
init() throws half-way through, and they could be deallocated gracefully.
Best regards,
-Stephan
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Implementing the sizes attribute of the link tag from HTML5
Hi, Darin Adler wrote: On Apr 15, 2010, at 3:41 PM, Aaron Boodman wrote: On Thu, Apr 15, 2010 at 3:25 PM, Maciej Stachowiak m...@apple.com wrote: Well, it's hard to truly have consensus on adding feature without knowing what it is. That being said, I at least would love to see a more specific proposal for what we could do with link size. I want to support bigger icons for the tabs in Chromium. I'm not sure what the path is for fetching favicons today. To load an icon you call retainIconForPageURL and then later call iconForPageURL once the icon is loaded. The iconForPageURL function takes an IntSize. But if you look at IconRecord.cpp you will see that at the moment the size isn’t yet used. Indeed. The method can return a BitmapImage instance, which may contain several bitmaps of different size. In the Haiku port, I've temporarily made some useful BitmapImage methods public, to iterate over all images contained in the object and find the one with the size I want (in the browser code later on). Is there a better way to do this? How do other browser projects handle this? Would a patch to make some BitmapImage methods public have good chances of being accepted or is there another vision? Best regards, -Stephan ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
[webkit-dev] How are discussions about patches working on bugzilla?
Hi all, so far I've made the experience that if patches received an initial review with r-, then if I give further feedback on the patch and try to discuss the best way to resolve it, I am not receiving any more comments on the ticket unless I actually attach a new patch. Am I supposed to change the review flag back to r? to get additional feedback for the same patch? So far I thought that all people in the bug's CC list receive email notifications for additional comments. Is this not the case? Here are some specific bugs where I am waiting for feedback: https://bugs.webkit.org/show_bug.cgi?id=35288 https://bugs.webkit.org/show_bug.cgi?id=34683 best regards, -Stephan ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Archiving the Haiku port? (was WebKit2 and all that jazz)
Hi, On 2010-04-10 at 00:26:36 [+0200], Maxime Simon maxime.si...@webkit.org wrote: I'm officially the main maintainer of the Haiku port. But considering my university course I have little time to work on it. (I had plan to do so next summer though.) Another Haiku developer, Stephan Assmus, did lately some great improvements on the port, but is now focused on the web browser and keeps a not up-to-date version of WebKit for his work. I've already upgraded one of my repositories to the latest SVN and resolved the conflicts and incorporated the changes. I just need to merge that with some changes in my other repository, but it's not a problem and won't take long. In fact, our community is really interested on this port but having a web browser seems to be of greater priority. I've been improving the port as well as working on the browser. A lot of work had to be done in the WebKit support layer and the Haiku WebKit API, it's just that the patches are mostly more than 20K per individual file I reckon... Best regards, -Stephan ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Archiving the Haiku port? (was WebKit2 and all that jazz)
- Ursprüngliche Nachricht - Von: Kenneth Christiansen Gesendet: 09.04.10 23:52 Uhr An: Adam Barth Betreff: Re: [webkit-dev] Archiving the Haiku port? (was WebKit2 and all that jazz) Last I looked there are still patches up for review for Haiku. It would be nice to know why it is not being maintained. Is that due to having no interest from their community or due to the fact that noone are reviewing the patches? Some people do review the patches, but the initial reviews appear usually days after I submitted a patch and sometimes there is no reaction after I update the patch or explain things. It is totally understandable... I don't mean to complain about reviewers, they are probably overwhelmed in work. However, this has resulted in me being reluctant about posting new patches. I would have a lot more patches in the pipe, but very few patches that would be about 20K or less. It has been expressed very clearly to me that only small patches are prefered. However, I have completely redesigned the Haiku WebKit API, implemented a great deal more code to expose additional WebCore features, done a lot of changes to the GraphicsContext implementation... I am under the impression that I would have to somehow fake individual changes to the files to somehow extract smaller patches. Like revert to the original file, add code back piece by piece while creating patches to keep them small. You can imagine it's a lot of additional work and overhead. I hope you guys understand that I've been reluctant to even try to get this stuff commited with how the review process went for me so far. However, I am willing to give it a try once more, if there are people interested in reviewing my patches and to keep the Haiku port going. It is certainly actively maintained. Best regards, -Stephan ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Archiving the Haiku port? (was WebKit2 and all that jazz)
- Ursprüngliche Nachricht - Von: Adam Barth Gesendet: 09.04.10 23:26 Uhr An: webkit-dev@lists.webkit.org Betreff: [webkit-dev] Archiving the Haiku port? (was WebKit2 and all that jazz) Is the Haiku port actively maintained? http://trac.webkit.org/browser/trunk/WebKit/haiku/ Looking at the ChangeLog, I don't see any real activity: http://trac.webkit.org/browser/trunk/WebKit/haiku/ChangeLog Maybe we should archive it? I certainly don't want to exclude Haiku from the community. Ideally, we'd make it easy to unarchive it if folks appear who want to work on it again. I am actively working on the Haiku port. I have submitted a number of patches, but since the review process takes _very_ long (no offense intended) I am careful to only submit patches when I don't anticipate changes to the files in the near future. A few of my patches still just sit there in bugzilla. Some got reviewed, the last activity was on a patch to ImageBufferHaiku.cpp, where the reviewer gave an r- because I removed the original copyright. Myself and Maxime Simone (original copyright holder) explained the issue and why it was correct to remove it. However, no response from the reviewer since then. I have a *huge* number of additional patches, since I am writing a WebKit browser for Haiku, completing the port, the Haiku WebKit API and the browser. Many of the patches would simply not be small, and to be frank, the review process has so far discouraged me from even trying to submit any of the bigger stuff. Especially the stuff I am working on all the time. I can certainly prepare updated patches and new patches in the next days, but it takes careful preparation and if those patches just sit there in bugzilla and receive no further feedback after initial review... it is just not motivating if the initial review came after about 10 days in the first place. The problem is of course that there are no dedicated Haiku reviewers. Other ports have their own number of reviewers and people interested in getting patches for their ports landed. With Haiku it's a chicken and egg problem. As long as I don't get a significant amount of patches landed, I won't even become committer. I don't really know what a solution could be. The Haiku port touches no other code, it's pretty self-contained. In that respect the chances are very low to affect other ports/people. I don't know if that could be an argument for just rubber stamping a lot of patches until the Haiku code in the official repo includes most of my stuff. What do you guys think? IMHO, the problem with the Haiku port is certainly not that it is not actively maintained, I work many hours on it each day, the problem is how to get the patches into SVN smoothly. Best regards, -Stephan ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] gdb archer and tracking live objects
On 2010-03-15 at 16:26:23 [+0100], Holger Freyther ze...@selfish.org wrote: On Monday 15 March 2010 15:47:43 you wrote: On Mon, Mar 15, 2010 at 12:38 AM, Holger Freyther ze...@selfish.org wrote: I wonder if I could develop this tool inside a subdirectory WebKitTools/Scripts without the usual review process? See my recent mail to webkit-dev about gdb 7 -- there's now a WebKitTools/gdb directory. I'm not sure what is different about your project that makes it so it would not benefit from code review. There are two things. In one way I want to have version control, and if the script turns out to be useful it should live inside the webkit tree but currently on the other it is anything but mature, it is me playing around and see what I can do it, and which approach is going to work nicely.. so for the highly experimental stuff it is a bit weird to attempt to convince another reviewer when I'm not convinced myself. It doesn't have to be about convincing, you may just get useful feedback. Best regards, -Stephan ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
[webkit-dev] CookieJar needs client interface?
Hi all, I am currently implementing the cookie jar backend for the Haiku port and browser. Looking at the other implementations, most of them do it in their network platform code, many are still incomplete. Isn't handling of cookies something browser specific? Looking at the Qt port, it seems like a drastic layering violation, using classes from WebKit in WebCore. So I was wondering if this shouldn't be implemented in a similar pattern to other client interfaces. With a CookieJarClient which has to be implemented in WebKit support code, thus allowing a WebKit client specific change of behavior with a clean separation of layers. Is this a good line of thought and is there any interest in something like this? I have set up build environments for the GTK port, Haiku of course and I could also get the Qt port compiling. So that I could try to provide implementations for at least these platforms. Another approach could be to provide the implementation for Haiku only, so that other ports could adopt this scheme if and when they want. Assuming of course that this change is at all desirable. Best regards, -Stephan ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] FontPlatformData, FontCache and HashMap
On 2010-02-21 at 06:02:01 [+0100], n179911 n179...@gmail.com wrote: I have a related question about SimpleFontData and FontPlatformData. When does Webkit create a FontPlatformData and SimpleFontData? I too put debug printfs in these classes constructors. And I see Webkit creates different FontPlatformData and SimpleFontData for same font family and same font size. It's all in FontCache.cpp. There is two levels of caching, that's why you see two FontPlatformData objects being created. One is created via the createFontPlatformData() that you have to implement. This object is placed in the hash map gFontPlatformDataCache in the method FontCache::getCachedFontPlatformData(). SimpleFontData objects are created in FontCache::getCachedFontData, which use the previously created FontPlatformData in the FontPlatformData copy constructor that you have to write, for the SimpleFontData member m_platformData. This SimpleFontData gets put into the gFontDataCache hash map and from it you will later retrieve the native font. FontPlatformData should to be implemented such that it maintains an internal object which wraps your platform native font with reference counting, so that copy consructor, operator=() and operator==() become cheap operations. The Qt implementation makes it clear how this has to work. What is also important is that you generate the same hash value for two different instances of your private native font wrapper which present the same logical font. Best regards, -Stephan ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] are there any known or suspected memory issues with webkit?
On 2010-02-21 at 13:14:59 [+0100], Mike Marchywka marchy...@hotmail.com wrote: The reason I ask is because I thought there were some concerns about leaks ( probably just stuff I saw skimming various google hits ) and I have seen firefox and iceweasel light up my disk on very simple things ( like typing stuff into forms). As I started looking through the code, one of the first things I saw was something that looked like a hash table of previously used pages- are there static junk bins of strong references and stuff like that that could create memory leaks as they accumulate stuff and never get cleaned or pruned? The other thing is that IMO ( opposing views welcome) memory access patterns are often an unappreciated issue on many architectures. The reason I reacted to the threading question as I did is that it seems, again in my opinion on superficial/ anecdotal analysis, that this approach often seems attractive but contains a number of issues, among them those related to memory or other resource allocations, that reduce the effectiveness even with an ideal multi-CPU system I did make this point before with reference to one speed-versus-threads graph from IEEE, http://lists.boost.org/boost-users/2008/11/42263.php Ceratinly your mileage may vary but resource allocation and contention is probably already a big deal and it may be a better place to look for optimization. Launching a thread is a general idea that can be used anywhere but digging into code may be more effort and more specialized to a given task however. Thoughts? I don't understand the multi-threading nay-saying. Just as an L2 cache is a technology to speed up memory access, multi-core architecture is a way to increase processing speed of the CPU. Last time I looked, a -j2 build of WebKit runs almost twice as fast as a single job build on my Core2Duo system. So the technology obviously works. Just because improving cache locality is a way to improve the execution speed of a piece of software, it doesn't mean one should try to prevent efforts to make software explore the increased processing performance of additional CPU cores. And designing the software to make proper use of multi-threading and scale well is definitely not easy, or the cheap way out. Not at all. Best regards, -Stephan ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] parallel rendering in WebKit
Hi, On 2010-02-19 at 23:46:11 [+0100], Benbuck Nason bna...@netflix.com wrote: To me it seems like the biggest gain would be to have two threads: one for update/layout, and another for rendering. This would probably require double-buffering the render state though. it's not really my place to jump into discussions about WebKit internals, since I am very new to the project, but I have some experience with this problem. By double-buffering the render state, do you mean to have the document tree in two copies? Or do you mean double buffering the graphics? The first is what needs to happen, since you want to render the document at a consistent point in time. Once this separation is done, you can even use as many rendering threads as you want, i.e. split the output bitmap into areas and use one thread per area. Other ways of splitting are possible too, like rendering independent compositing layers in different threads. But one needs a point in time to take a snapshot of the current document tree, kick of the rendering of this snapshot, while the main document is allowed to evolve. I can't really say if such a design makes sense for WebKit. Depending on the architecture of a port, rendering may already take place in another thread. for example, when drawing commands are dispatched to the GPU. Or in the Haiku port, drawing commands are dispatched to a system service, which does the actual work. Synchronization happens when the contents are blitted to the screen. What would be nice is if each page could run it's own layout thread. But AFAICT, a lot of work would need to be done for this. When I started to pick up work on the Haiku port, I had to redesign the threading, and make sure I call into WebCore code only on the main thread. There are tons of assertions to make sure of this (nice for porting). But even nicer would be a concept of threading and locking critical sections, instead of asserting a specific thread. Maybe this doesn't make so much sense for WebKit and I am just showing off my newbie status. :-) Best regards, -Stephan ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
[webkit-dev] FontPlatformData, FontCache and HashMap
Hi all,
currently, I am investigating some weirdness with regards to
FontPlatformData and the FontCache which I see in my debugging output on
the Haiku port. I am not even 100% sure that what I see is a weirdness, but
at least I don't understand it and I was wondering if anyone could help me
shed some light into this.
So I understand these things:
1) SimpleFontData has a member m_platformData of type FontPlatformData,
which is always instantiated by passing it a const reference of another
FontPlatformData.
2) The FontCache maintains SimplaFontData instances in a HashMap, and looks
these up and compares them by using FontPlatformData objects as keys into
the map.
3) FontCache has a static empty FontPlatformData instance in
FontDataCacheKeyTraits:
static const FontPlatformData emptyValue()
{
DEFINE_STATIC_LOCAL(FontPlatformData, key, (0.f, false, false));
return key;
}
Don't yet know how this comes into play, but possibly it's of relevance.
My analysis is a bit awkward, since I cannot use a source level debugger on
Haiku, and I won't bore you with the embarrasing details of why. So I added
printf() statements in all my constructors of FontPlatformData, and printed
among other things the memory address of the FontPlatformData instance
(this). Also, I am printing this in a number of other places, operator=()
for example. The output I am getting is a little weird, since it shows that
operator=() is called on objects, for which no constructor has been called.
Basically, I've narrowed it down to SimpleFontData*
FontCache::getCachedFontData(const FontPlatformData* platformData) in
FontCache.cpp. In this method, a new SimpleFontData instance is to be
created and inserted into the HashMap. I will show you the output from the
program start. It printed which FontPlatformData instances have been
created up to Kill Thread (I let it drop into the debugger from the first
invokation of FontPlatformData::operator=()):
The first font is created:
getCachedFontPlatformData(0)
createFontPlatformData(const FontDescription)
0x18018660-FontPlatformData(const FontDescription(16.0, Myriad Pro))
0x18019bf0-FontPlatformDataPrivate()
created result: 0x18018660
getCachedFontPlatformData() - done: 0x18018660
Next, getCachedFontData() is called to insert a new SimpleFontData for this
FontDescription:
getCachedFontData(0x18018660)
Next comes the output from line 289 of FontCache.cpp:
pairSimpleFontData*, unsigned newValue(new SimpleFontData(*platformData),
1);
A new SimpleFontData object is created with it's m_fontData member at
address 0x2f4c6a8:
0x2f4c6a8-FontPlatformData(const FontPlatformData 0x18018660)
0x18019bf0-addRef()
new: 0x2f4c680
Next comes the call to HashMap::set(), which results in this output:
0x18018970-FontPlatformData(0.0, 0, 0)
This is probably the key created from HashMap::inlineAdd(). And next,
HashMap::set() invokes operator=() on the key (line 250) which gives this
output:
0x2ec8594 ((nil), 0.0, 0, 0) -operator=(0x18018660, 0x18019bf0)
nil is a pointer to FontPlatformDataPrivate, which is ok. But 0x2ec8594
should be the pointer to the FontPlatformData instance created in
inlineAdd().
How can I be doing something wrong there? Nothing happens in my constructor
for FontPlatformData() when the empty key values are used. Maybe the usage
of the HashMap is broken in FontCache, since it would seem that such a
low-level utility class as HashMap would not likely contain any bugs.
Could someone help me shed some light on this? Thanks a bunch for reading
this far!
Best regards,
-Stephan
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] FontPlatformData, FontCache and HashMap
On 2010-02-18 at 18:43:08 [+0100], Stephan Assmus supersti...@gmx.de wrote:
Hi all,
currently, I am investigating some weirdness with regards to
FontPlatformData and the FontCache which I see in my debugging output on
the Haiku port. I am not even 100% sure that what I see is a weirdness, but
at least I don't understand it and I was wondering if anyone could help me
shed some light into this.
So I understand these things:
1) SimpleFontData has a member m_platformData of type FontPlatformData,
which is always instantiated by passing it a const reference of another
FontPlatformData.
2) The FontCache maintains SimplaFontData instances in a HashMap, and looks
these up and compares them by using FontPlatformData objects as keys into
the map.
3) FontCache has a static empty FontPlatformData instance in
FontDataCacheKeyTraits:
static const FontPlatformData emptyValue()
{
DEFINE_STATIC_LOCAL(FontPlatformData, key, (0.f, false, false));
return key;
}
Don't yet know how this comes into play, but possibly it's of relevance.
My analysis is a bit awkward, since I cannot use a source level debugger on
Haiku, and I won't bore you with the embarrasing details of why. So I added
printf() statements in all my constructors of FontPlatformData, and printed
among other things the memory address of the FontPlatformData instance
(this). Also, I am printing this in a number of other places, operator=()
for example. The output I am getting is a little weird, since it shows that
operator=() is called on objects, for which no constructor has been called.
Basically, I've narrowed it down to SimpleFontData*
FontCache::getCachedFontData(const FontPlatformData* platformData) in
FontCache.cpp. In this method, a new SimpleFontData instance is to be
created and inserted into the HashMap. I will show you the output from the
program start. It printed which FontPlatformData instances have been
created up to Kill Thread (I let it drop into the debugger from the first
invokation of FontPlatformData::operator=()):
The first font is created:
getCachedFontPlatformData(0)
createFontPlatformData(const FontDescription)
0x18018660-FontPlatformData(const FontDescription(16.0, Myriad Pro))
0x18019bf0-FontPlatformDataPrivate()
created result: 0x18018660
getCachedFontPlatformData() - done: 0x18018660
Next, getCachedFontData() is called to insert a new SimpleFontData for this
FontDescription:
getCachedFontData(0x18018660)
Next comes the output from line 289 of FontCache.cpp:
pairSimpleFontData*, unsigned newValue(new SimpleFontData(*platformData),
1);
A new SimpleFontData object is created with it's m_fontData member at
address 0x2f4c6a8:
0x2f4c6a8-FontPlatformData(const FontPlatformData 0x18018660)
0x18019bf0-addRef()
new: 0x2f4c680
Next comes the call to HashMap::set(), which results in this output:
0x18018970-FontPlatformData(0.0, 0, 0)
This is probably the key created from HashMap::inlineAdd(). And next,
HashMap::set() invokes operator=() on the key (line 250) which gives this
output:
0x2ec8594 ((nil), 0.0, 0, 0) -operator=(0x18018660, 0x18019bf0)
nil is a pointer to FontPlatformDataPrivate, which is ok. But 0x2ec8594
should be the pointer to the FontPlatformData instance created in
inlineAdd().
How can I be doing something wrong there? Nothing happens in my constructor
for FontPlatformData() when the empty key values are used. Maybe the usage
of the HashMap is broken in FontCache, since it would seem that such a
low-level utility class as HashMap would not likely contain any bugs.
Ok, I've finally figured it out. FontCache.cpp defines the following for use
in the HashMap:
struct FontDataCacheKeyTraits : WTF::GenericHashTraitsFontPlatformData {
static const bool emptyValueIsZero = true;
static const bool needsDestruction = true;
static const FontPlatformData emptyValue()
{
DEFINE_STATIC_LOCAL(FontPlatformData, key, (0.f, false, false));
return key;
}
static void constructDeletedValue(FontPlatformData slot)
{
new (slot) FontPlatformData(HashTableDeletedValue);
}
static bool isDeletedValue(const FontPlatformData value)
{
return value.isHashTableDeletedValue();
}
};
The emptyValueIsZero means the HashTable implementation will not call
constructors for new slots. IMHO, this is a dangerous assumption, since the
porters may not realize this and that this actually works is completely
implementation specific.
Best regards,
-Stephan
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Webkit mipsel crashing in arenaDelete
Hi, On 2010-02-17 at 11:32:57 [+0100], Bharathwaaj S bharathwaa...@gmail.com wrote: I tried the same as mentioned by Stephan. #if !defined(USE_SYSTEM_MALLOC) defined(NDEBUG) #define FORCE_SYSTEM_MALLOC 0 #else #define FORCE_SYSTEM_MALLOC 0 #endif I might have led you onto the wrong track. Or maybe it is the right track to explain your crashing issues (the allocator), but my solution is probably wrong. First of all, I have to explain that I am not the developer debugging the allocator issues on the Haiku port. This is what I understand: We did remove the USE_SYSTEM_MALLOC=1 define from our build system. We ran debug builds through a special version of Haiku's libroot.so which has an allocator with debugging facilities. As soon as USE_SYSTEM_MALLOC=1 was not defined anymore, the JavaScriptCore allocations did not show up anymore in this debugging allocator. However, later I have found out that wtf/Platform.h still defines USE_SYSTEM_MALLOC=1 for the Haiku port. So I don't know why (changing) the defines via our build system would have any affect at all. Maybe only changing FORCE_SYSTEM_MALLOC above actually had an effect, but it should only affect debug builds. Finally, the Haiku port suffers random crashes in the JavaScriptCore code with regards to allocation at the moment (release/debug doesn't matter). We are working on the issue and as soon as I know more, I'll let you know. Second, I've found out that even though ENABLE_JSC_MULTIPLE_THREADS is not defined for Haiku, compiling TCSystemMalloc into the build will result in a scavenger thread running to periodically reclaim unused allocations (using the PThread backend). But ENABLE_JSC_MULTIPLE_THREADS is actually only supported on Windows and Mac. So I have no idea if that was healthy at all and I disabled the scavenger thread, since I presume if the rest of the code assumes to run single threaded, another thread doing garbage collection in parallel may result in havoc. Still, this doesn't fix the random crashes of the Haiku port, which is why I believe we may have some screwup with regards to which allocator is being used at which time. It could as well be misaligned free()s that we saw, due to wrong assumptions in JSC when using the system allocator. Will let you know when I know more, but maybe the above can give you some helpful pointers what things to look into and confirm for your port. Best regards, -Stephan ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Webkit mipsel crashing in arenaDelete
Hi, On 2010-02-16 at 08:50:21 [+0100], Bharathwaaj S bharathwaa...@gmail.com wrote: I could port webkit to mipsel architechture using DirectFB Backend. I've taken svn WebKitGtk release 1.1.8 present herehttp://trac.webkit.org/browser/releases/WebKitGTK/webkit-1.1.8 . I could get google.com homepage when I run GtkLauncher. But when I press any the Google Search button, it crashes. I added printfs in the code and I found that it is crashing in File WebCore/rendering/RenderObject.cpp Function arenaDelete in the line delete this It also crashes when I reload the google page by pressing right click and reload. The same release when built for x86 runs fine. Kindly help me in solving this issue. No idea if this is helpful or even applies to your problem, but during the Haiku porting effort, we found out that when using USE_SYSTEM_MALLOC=1, WebCore makes some assumptions about alignment of allocations that are not guarenteed when using malloc() (should use memalign()). When we switched to using the built-in TCSystemMalloc, those problems went away. Also, I don't know which build system you are using, but you need to make sure that you compile all parts of WebKit with the same defines. Since there are a lot of #defines in the code which enable or disable various features, the object sizes will change when you compile different parts of the code with different #defines, which can lead to all sorts of funny crashes. Best regards, -Stephan ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Webkit mipsel crashing in arenaDelete
Hi, On 2010-02-16 at 09:31:32 [+0100], İsmail Dönmez ism...@namtrac.org wrote: On Tue, Feb 16, 2010 at 10:22 AM, Stephan Assmus supersti...@gmx.de wrote: Hi, On 2010-02-16 at 08:50:21 [+0100], Bharathwaaj S bharathwaa...@gmail.com wrote: I could port webkit to mipsel architechture using DirectFB Backend. I've taken svn WebKitGtk release 1.1.8 present herehttp://trac.webkit.org/browser/releases/WebKitGTK/webkit-1.1.8 . I could get google.com homepage when I run GtkLauncher. But when I press any the Google Search button, it crashes. I added printfs in the code and I found that it is crashing in File WebCore/rendering/RenderObject.cpp Function arenaDelete in the line delete this It also crashes when I reload the google page by pressing right click and reload. The same release when built for x86 runs fine. Kindly help me in solving this issue. No idea if this is helpful or even applies to your problem, but during the Haiku porting effort, we found out that when using USE_SYSTEM_MALLOC=1, WebCore makes some assumptions about alignment of allocations that are not guarenteed when using malloc() (should use memalign()). When we switched to using the built-in TCSystemMalloc, those problems went away. Thats interesting. How did you force TCSystemMalloc for Haiku port? I could send you a diff for just this change, but the Haiku port uses it's own buildsystem which is not (yet?) part of WebKit trunk. All that seemed necessary to enable it was to 1) not define USE_SYSTEM_MALLOC=1 2) include TCSystemAlloc.cpp into the libjavascriptcore.so build. For debug builds, we also force the use of system malloc to OFF: Index: JavaScriptCore/wtf/FastMalloc.cpp === --- JavaScriptCore/wtf/FastMalloc.cpp (revision 47) +++ JavaScriptCore/wtf/FastMalloc.cpp (revision 49) @@ -92,7 +92,7 @@ #if !(defined(USE_SYSTEM_MALLOC) USE_SYSTEM_MALLOC) defined(NDEBUG) #define FORCE_SYSTEM_MALLOC 0 #else -#define FORCE_SYSTEM_MALLOC 1 +#define FORCE_SYSTEM_MALLOC 0 // TODO: hacked to off also in debug build #endif // Use a background thread to periodically scavenge memory to release back to the system This was necessary for some reason I cannot remember right now. Best regards, -Stephan ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
[webkit-dev] Asking porting related questions
Hi all, since a couple days, I have picked up work on the Haiku port of WebKit and have already gotten pretty far and submitted patches and all. However, sometimes I've gotten stuck on specific subjects and asked questions on webkit-help, which I believed is the appropriate list for asking porting related questions. But so far my more specific quesions seem ignored. :-) Being invovled in other open source projects, I know the reason may simply be that nobody on webkit-help feels she/he can give a good answer. So I am wondering if it's ok to ask specific platform related questions also on this list, since the website mentions webkit-help specifically for porting related questions. Best regards, -Stephan ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
