[webkit-changes] [164185] trunk

Sat, 15 Feb 2014 18:50:26 -0800

Title: [164185] trunk
Revision
164185
Author
[email protected]
Date
2014-02-15 18:54:26 -0800 (Sat, 15 Feb 2014)

Log Message

Vector with inline capacity should work with non-PODs
https://bugs.webkit.org/show_bug.cgi?id=128864

Source/_javascript_Core: 

Reviewed by Michael Saboff.
        
Deques no longer have inline capacity because it was broken, and we didn't need it
here anyway.

* dfg/DFGWorklist.h:

Source/WebCore: 

Reviewed by Michael Saboff.

No new tests because no change behavior.
        
Deques no longer have inline capacity because it was broken, and we didn't need it
here anyway.

* page/WheelEventDeltaTracker.h:

Source/WTF: 

Reviewed by Michael Saboff.
        
Previously, we would copy the inline storage of a vector as if it was a bag of bits.
This presumed that the element type was relocatable. In general this is only true for
PODs.
        
This patch changes this by introducing a swap operation over inline storage. This swap
operation requires being told about the size that is in use.
        
Deques would have required some cleverness to make this work, because the swap
operation needs to know which subset of elements are in-use and assumes that a size is
sufficient for this. That's not true for deques. Instead of trying to do very clever
things, I just removed the inline capacity option from Deque. I believe that this is
fine since there are only two users of Deque with inline capacity, and both of them
appear to be allocated rarely enough that inline capacity probably doesn't help much.

* wtf/Deque.h:
(WTF::DequeIterator::DequeIterator):
(WTF::DequeConstIterator::DequeConstIterator):
(WTF::Deque<T>::checkValidity):
(WTF::Deque<T>::checkIndexValidity):
(WTF::Deque<T>::invalidateIterators):
(WTF::Deque<T>::Deque):
(WTF::=):
(WTF::Deque<T>::destroyAll):
(WTF::Deque<T>::~Deque):
(WTF::Deque<T>::swap):
(WTF::Deque<T>::clear):
(WTF::Deque<T>::expandCapacityIfNeeded):
(WTF::Deque<T>::expandCapacity):
(WTF::Deque<T>::append):
(WTF::Deque<T>::prepend):
(WTF::Deque<T>::removeFirst):
(WTF::Deque<T>::removeLast):
(WTF::Deque<T>::remove):
(WTF::DequeIteratorBase<T>::checkValidity):
(WTF::DequeIteratorBase<T>::addToIteratorsList):
(WTF::DequeIteratorBase<T>::removeFromIteratorsList):
(WTF::DequeIteratorBase<T>::DequeIteratorBase):
(WTF::DequeIteratorBase<T>::~DequeIteratorBase):
(WTF::DequeIteratorBase<T>::isEqual):
(WTF::DequeIteratorBase<T>::increment):
(WTF::DequeIteratorBase<T>::decrement):
(WTF::DequeIteratorBase<T>::after):
(WTF::DequeIteratorBase<T>::before):
* wtf/Vector.h:
(WTF::VectorBuffer::swap):
(WTF::VectorBuffer::swapInlineBuffer):
(WTF::VectorBuffer::swapInlineBuffers):
(WTF::Vector::swap):

Tools: 

Reviewed by Michael Saboff.
        
This test experiences really bizarre behavior on trunk without the rest of
this fix. On my machine, it usually times out because it gets itself into an
infinite loop of some kind. With the fix, it passes.

* TestWebKitAPI/Tests/WTF/Vector.cpp:
(TestWebKitAPI::TEST):

Modified Paths

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (164184 => 164185)


--- trunk/Source/_javascript_Core/ChangeLog	2014-02-16 02:01:04 UTC (rev 164184)
+++ trunk/Source/_javascript_Core/ChangeLog	2014-02-16 02:54:26 UTC (rev 164185)
@@ -1,5 +1,17 @@
 2014-02-15  Filip Pizlo  <[email protected]>
 
+        Vector with inline capacity should work with non-PODs
+        https://bugs.webkit.org/show_bug.cgi?id=128864
+
+        Reviewed by Michael Saboff.
+        
+        Deques no longer have inline capacity because it was broken, and we didn't need it
+        here anyway.
+
+        * dfg/DFGWorklist.h:
+
+2014-02-15  Filip Pizlo  <[email protected]>
+
         Unreviewed, roll out r164166.
 
         This broke three unique tests:

Modified: trunk/Source/_javascript_Core/dfg/DFGWorklist.h (164184 => 164185)


--- trunk/Source/_javascript_Core/dfg/DFGWorklist.h	2014-02-16 02:01:04 UTC (rev 164184)
+++ trunk/Source/_javascript_Core/dfg/DFGWorklist.h	2014-02-16 02:54:26 UTC (rev 164185)
@@ -89,7 +89,7 @@
     void dump(const MutexLocker&, PrintStream&) const;
 
     // Used to inform the thread about what work there is left to do.
-    Deque<RefPtr<Plan>, 16> m_queue;
+    Deque<RefPtr<Plan>> m_queue;
     
     // Used to answer questions about the current state of a code block. This
     // is particularly great for the cti_optimize OSR slow path, which wants

Modified: trunk/Source/WTF/ChangeLog (164184 => 164185)


--- trunk/Source/WTF/ChangeLog	2014-02-16 02:01:04 UTC (rev 164184)
+++ trunk/Source/WTF/ChangeLog	2014-02-16 02:54:26 UTC (rev 164185)
@@ -1,3 +1,59 @@
+2014-02-15  Filip Pizlo  <[email protected]>
+
+        Vector with inline capacity should work with non-PODs
+        https://bugs.webkit.org/show_bug.cgi?id=128864
+
+        Reviewed by Michael Saboff.
+        
+        Previously, we would copy the inline storage of a vector as if it was a bag of bits.
+        This presumed that the element type was relocatable. In general this is only true for
+        PODs.
+        
+        This patch changes this by introducing a swap operation over inline storage. This swap
+        operation requires being told about the size that is in use.
+        
+        Deques would have required some cleverness to make this work, because the swap
+        operation needs to know which subset of elements are in-use and assumes that a size is
+        sufficient for this. That's not true for deques. Instead of trying to do very clever
+        things, I just removed the inline capacity option from Deque. I believe that this is
+        fine since there are only two users of Deque with inline capacity, and both of them
+        appear to be allocated rarely enough that inline capacity probably doesn't help much.
+
+        * wtf/Deque.h:
+        (WTF::DequeIterator::DequeIterator):
+        (WTF::DequeConstIterator::DequeConstIterator):
+        (WTF::Deque<T>::checkValidity):
+        (WTF::Deque<T>::checkIndexValidity):
+        (WTF::Deque<T>::invalidateIterators):
+        (WTF::Deque<T>::Deque):
+        (WTF::=):
+        (WTF::Deque<T>::destroyAll):
+        (WTF::Deque<T>::~Deque):
+        (WTF::Deque<T>::swap):
+        (WTF::Deque<T>::clear):
+        (WTF::Deque<T>::expandCapacityIfNeeded):
+        (WTF::Deque<T>::expandCapacity):
+        (WTF::Deque<T>::append):
+        (WTF::Deque<T>::prepend):
+        (WTF::Deque<T>::removeFirst):
+        (WTF::Deque<T>::removeLast):
+        (WTF::Deque<T>::remove):
+        (WTF::DequeIteratorBase<T>::checkValidity):
+        (WTF::DequeIteratorBase<T>::addToIteratorsList):
+        (WTF::DequeIteratorBase<T>::removeFromIteratorsList):
+        (WTF::DequeIteratorBase<T>::DequeIteratorBase):
+        (WTF::DequeIteratorBase<T>::~DequeIteratorBase):
+        (WTF::DequeIteratorBase<T>::isEqual):
+        (WTF::DequeIteratorBase<T>::increment):
+        (WTF::DequeIteratorBase<T>::decrement):
+        (WTF::DequeIteratorBase<T>::after):
+        (WTF::DequeIteratorBase<T>::before):
+        * wtf/Vector.h:
+        (WTF::VectorBuffer::swap):
+        (WTF::VectorBuffer::swapInlineBuffer):
+        (WTF::VectorBuffer::swapInlineBuffers):
+        (WTF::Vector::swap):
+
 2014-02-15  Mikhail Pozdnyakov  <[email protected]>
 
         Remove 'static' specifier from free inline functions in StringImpl.h

Modified: trunk/Source/WTF/wtf/Deque.h (164184 => 164185)


--- trunk/Source/WTF/wtf/Deque.h	2014-02-16 02:01:04 UTC (rev 164184)
+++ trunk/Source/WTF/wtf/Deque.h	2014-02-16 02:54:26 UTC (rev 164185)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2007, 2008 Apple Inc. All rights reserved.
+ * Copyright (C) 2007, 2008, 2014 Apple Inc. All rights reserved.
  * Copyright (C) 2009 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -38,25 +38,25 @@
 
 namespace WTF {
 
-    template<typename T, size_t inlineCapacity> class DequeIteratorBase;
-    template<typename T, size_t inlineCapacity> class DequeIterator;
-    template<typename T, size_t inlineCapacity> class DequeConstIterator;
+    template<typename T> class DequeIteratorBase;
+    template<typename T> class DequeIterator;
+    template<typename T> class DequeConstIterator;
 
-    template<typename T, size_t inlineCapacity = 0>
+    template<typename T>
     class Deque {
         WTF_MAKE_FAST_ALLOCATED;
     public:
-        typedef DequeIterator<T, inlineCapacity> iterator;
-        typedef DequeConstIterator<T, inlineCapacity> const_iterator;
+        typedef DequeIterator<T> iterator;
+        typedef DequeConstIterator<T> const_iterator;
         typedef std::reverse_iterator<iterator> reverse_iterator;
         typedef std::reverse_iterator<const_iterator> const_reverse_iterator;
 
         Deque();
-        Deque(const Deque<T, inlineCapacity>&);
-        Deque& operator=(const Deque<T, inlineCapacity>&);
+        Deque(const Deque<T>&);
+        Deque& operator=(const Deque<T>&);
         ~Deque();
 
-        void swap(Deque<T, inlineCapacity>&);
+        void swap(Deque<T>&);
 
         size_t size() const { return m_start <= m_end ? m_end - m_start : m_end + m_buffer.capacity() - m_start; }
         bool isEmpty() const { return m_start == m_end; }
@@ -91,11 +91,11 @@
         iterator findIf(Predicate&&);
 
     private:
-        friend class DequeIteratorBase<T, inlineCapacity>;
+        friend class DequeIteratorBase<T>;
 
-        typedef VectorBuffer<T, inlineCapacity> Buffer;
+        typedef VectorBuffer<T, 0> Buffer;
         typedef VectorTypeOperations<T> TypeOperations;
-        typedef DequeIteratorBase<T, inlineCapacity> IteratorBase;
+        typedef DequeIteratorBase<T> IteratorBase;
 
         void remove(size_t position);
         void invalidateIterators();
@@ -113,11 +113,11 @@
 #endif
     };
 
-    template<typename T, size_t inlineCapacity = 0>
+    template<typename T>
     class DequeIteratorBase {
     protected:
         DequeIteratorBase();
-        DequeIteratorBase(const Deque<T, inlineCapacity>*, size_t);
+        DequeIteratorBase(const Deque<T>*, size_t);
         DequeIteratorBase(const DequeIteratorBase&);
         DequeIteratorBase& operator=(const DequeIteratorBase&);
         ~DequeIteratorBase();
@@ -138,10 +138,10 @@
         void checkValidity() const;
         void checkValidity(const DequeIteratorBase&) const;
 
-        Deque<T, inlineCapacity>* m_deque;
+        Deque<T>* m_deque;
         size_t m_index;
 
-        friend class Deque<T, inlineCapacity>;
+        friend class Deque<T>;
 
 #ifndef NDEBUG
         mutable DequeIteratorBase* m_next;
@@ -149,11 +149,11 @@
 #endif
     };
 
-    template<typename T, size_t inlineCapacity = 0>
-    class DequeIterator : public DequeIteratorBase<T, inlineCapacity> {
+    template<typename T>
+    class DequeIterator : public DequeIteratorBase<T> {
     private:
-        typedef DequeIteratorBase<T, inlineCapacity> Base;
-        typedef DequeIterator<T, inlineCapacity> Iterator;
+        typedef DequeIteratorBase<T> Base;
+        typedef DequeIterator<T> Iterator;
 
     public:
         typedef ptrdiff_t difference_type;
@@ -162,7 +162,7 @@
         typedef T& reference;
         typedef std::bidirectional_iterator_tag iterator_category;
 
-        DequeIterator(Deque<T, inlineCapacity>* deque, size_t index) : Base(deque, index) { }
+        DequeIterator(Deque<T>* deque, size_t index) : Base(deque, index) { }
 
         DequeIterator(const Iterator& other) : Base(other) { }
         DequeIterator& operator=(const Iterator& other) { Base::assign(other); return *this; }
@@ -179,12 +179,12 @@
         // postfix -- intentionally omitted
     };
 
-    template<typename T, size_t inlineCapacity = 0>
-    class DequeConstIterator : public DequeIteratorBase<T, inlineCapacity> {
+    template<typename T>
+    class DequeConstIterator : public DequeIteratorBase<T> {
     private:
-        typedef DequeIteratorBase<T, inlineCapacity> Base;
-        typedef DequeConstIterator<T, inlineCapacity> Iterator;
-        typedef DequeIterator<T, inlineCapacity> NonConstIterator;
+        typedef DequeIteratorBase<T> Base;
+        typedef DequeConstIterator<T> Iterator;
+        typedef DequeIterator<T> NonConstIterator;
 
     public:
         typedef ptrdiff_t difference_type;
@@ -193,7 +193,7 @@
         typedef const T& reference;
         typedef std::bidirectional_iterator_tag iterator_category;
 
-        DequeConstIterator(const Deque<T, inlineCapacity>* deque, size_t index) : Base(deque, index) { }
+        DequeConstIterator(const Deque<T>* deque, size_t index) : Base(deque, index) { }
 
         DequeConstIterator(const Iterator& other) : Base(other) { }
         DequeConstIterator(const NonConstIterator& other) : Base(other) { }
@@ -213,12 +213,12 @@
     };
 
 #ifdef NDEBUG
-    template<typename T, size_t inlineCapacity> inline void Deque<T, inlineCapacity>::checkValidity() const { }
-    template<typename T, size_t inlineCapacity> inline void Deque<T, inlineCapacity>::checkIndexValidity(size_t) const { }
-    template<typename T, size_t inlineCapacity> inline void Deque<T, inlineCapacity>::invalidateIterators() { }
+    template<typename T> inline void Deque<T>::checkValidity() const { }
+    template<typename T> inline void Deque<T>::checkIndexValidity(size_t) const { }
+    template<typename T> inline void Deque<T>::invalidateIterators() { }
 #else
-    template<typename T, size_t inlineCapacity>
-    void Deque<T, inlineCapacity>::checkValidity() const
+    template<typename T>
+    void Deque<T>::checkValidity() const
     {
         // In this implementation a capacity of 1 would confuse append() and
         // other places that assume the index after capacity - 1 is 0.
@@ -233,8 +233,8 @@
         }
     }
 
-    template<typename T, size_t inlineCapacity>
-    void Deque<T, inlineCapacity>::checkIndexValidity(size_t index) const
+    template<typename T>
+    void Deque<T>::checkIndexValidity(size_t index) const
     {
         ASSERT_UNUSED(index, index <= m_buffer.capacity());
         if (m_start <= m_end) {
@@ -245,8 +245,8 @@
         }
     }
 
-    template<typename T, size_t inlineCapacity>
-    void Deque<T, inlineCapacity>::invalidateIterators()
+    template<typename T>
+    void Deque<T>::invalidateIterators()
     {
         IteratorBase* next;
         for (IteratorBase* p = m_iterators; p; p = next) {
@@ -259,8 +259,8 @@
     }
 #endif
 
-    template<typename T, size_t inlineCapacity>
-    inline Deque<T, inlineCapacity>::Deque()
+    template<typename T>
+    inline Deque<T>::Deque()
         : m_start(0)
         , m_end(0)
 #ifndef NDEBUG
@@ -270,8 +270,8 @@
         checkValidity();
     }
 
-    template<typename T, size_t inlineCapacity>
-    inline Deque<T, inlineCapacity>::Deque(const Deque<T, inlineCapacity>& other)
+    template<typename T>
+    inline Deque<T>::Deque(const Deque<T>& other)
         : m_start(other.m_start)
         , m_end(other.m_end)
         , m_buffer(other.m_buffer.capacity())
@@ -288,8 +288,8 @@
         }
     }
 
-    template<typename T, size_t inlineCapacity>
-    inline Deque<T, inlineCapacity>& Deque<T, inlineCapacity>::operator=(const Deque<T, inlineCapacity>& other)
+    template<typename T>
+    inline Deque<T>& Deque<T>::operator=(const Deque<T>& other)
     {
         // FIXME: This is inefficient if we're using an inline buffer and T is
         // expensive to copy since it will copy the buffer twice instead of once.
@@ -298,8 +298,8 @@
         return *this;
     }
 
-    template<typename T, size_t inlineCapacity>
-    inline void Deque<T, inlineCapacity>::destroyAll()
+    template<typename T>
+    inline void Deque<T>::destroyAll()
     {
         if (m_start <= m_end)
             TypeOperations::destruct(m_buffer.buffer() + m_start, m_buffer.buffer() + m_end);
@@ -309,29 +309,29 @@
         }
     }
 
-    template<typename T, size_t inlineCapacity>
-    inline Deque<T, inlineCapacity>::~Deque()
+    template<typename T>
+    inline Deque<T>::~Deque()
     {
         checkValidity();
         invalidateIterators();
         destroyAll();
     }
 
-    template<typename T, size_t inlineCapacity>
-    inline void Deque<T, inlineCapacity>::swap(Deque<T, inlineCapacity>& other)
+    template<typename T>
+    inline void Deque<T>::swap(Deque<T>& other)
     {
         checkValidity();
         other.checkValidity();
         invalidateIterators();
         std::swap(m_start, other.m_start);
         std::swap(m_end, other.m_end);
-        m_buffer.swap(other.m_buffer);
+        m_buffer.swap(other.m_buffer, 0, 0);
         checkValidity();
         other.checkValidity();
     }
 
-    template<typename T, size_t inlineCapacity>
-    inline void Deque<T, inlineCapacity>::clear()
+    template<typename T>
+    inline void Deque<T>::clear()
     {
         checkValidity();
         invalidateIterators();
@@ -342,9 +342,9 @@
         checkValidity();
     }
 
-    template<typename T, size_t inlineCapacity>
+    template<typename T>
     template<typename Predicate>
-    inline auto Deque<T, inlineCapacity>::findIf(Predicate&& predicate) -> iterator
+    inline auto Deque<T>::findIf(Predicate&& predicate) -> iterator
     {
         iterator end_iterator = end();
         for (iterator it = begin(); it != end_iterator; ++it) {
@@ -354,8 +354,8 @@
         return end_iterator;
     }
 
-    template<typename T, size_t inlineCapacity>
-    inline void Deque<T, inlineCapacity>::expandCapacityIfNeeded()
+    template<typename T>
+    inline void Deque<T>::expandCapacityIfNeeded()
     {
         if (m_start) {
             if (m_end + 1 != m_start)
@@ -369,8 +369,8 @@
         expandCapacity();
     }
 
-    template<typename T, size_t inlineCapacity>
-    void Deque<T, inlineCapacity>::expandCapacity()
+    template<typename T>
+    void Deque<T>::expandCapacity()
     {
         checkValidity();
         size_t oldCapacity = m_buffer.capacity();
@@ -388,24 +388,24 @@
         checkValidity();
     }
 
-    template<typename T, size_t inlineCapacity>
-    inline auto Deque<T, inlineCapacity>::takeFirst() -> T
+    template<typename T>
+    inline auto Deque<T>::takeFirst() -> T
     {
         T oldFirst = std::move(first());
         removeFirst();
         return oldFirst;
     }
 
-    template<typename T, size_t inlineCapacity>
-    inline auto Deque<T, inlineCapacity>::takeLast() -> T
+    template<typename T>
+    inline auto Deque<T>::takeLast() -> T
     {
         T oldLast = std::move(last());
         removeLast();
         return oldLast;
     }
 
-    template<typename T, size_t inlineCapacity> template<typename U>
-    inline void Deque<T, inlineCapacity>::append(U&& value)
+    template<typename T> template<typename U>
+    inline void Deque<T>::append(U&& value)
     {
         checkValidity();
         expandCapacityIfNeeded();
@@ -417,8 +417,8 @@
         checkValidity();
     }
 
-    template<typename T, size_t inlineCapacity> template<typename U>
-    inline void Deque<T, inlineCapacity>::prepend(U&& value)
+    template<typename T> template<typename U>
+    inline void Deque<T>::prepend(U&& value)
     {
         checkValidity();
         expandCapacityIfNeeded();
@@ -430,8 +430,8 @@
         checkValidity();
     }
 
-    template<typename T, size_t inlineCapacity>
-    inline void Deque<T, inlineCapacity>::removeFirst()
+    template<typename T>
+    inline void Deque<T>::removeFirst()
     {
         checkValidity();
         invalidateIterators();
@@ -444,8 +444,8 @@
         checkValidity();
     }
 
-    template<typename T, size_t inlineCapacity>
-    inline void Deque<T, inlineCapacity>::removeLast()
+    template<typename T>
+    inline void Deque<T>::removeLast()
     {
         checkValidity();
         invalidateIterators();
@@ -458,22 +458,22 @@
         checkValidity();
     }
 
-    template<typename T, size_t inlineCapacity>
-    inline void Deque<T, inlineCapacity>::remove(iterator& it)
+    template<typename T>
+    inline void Deque<T>::remove(iterator& it)
     {
         it.checkValidity();
         remove(it.m_index);
     }
 
-    template<typename T, size_t inlineCapacity>
-    inline void Deque<T, inlineCapacity>::remove(const_iterator& it)
+    template<typename T>
+    inline void Deque<T>::remove(const_iterator& it)
     {
         it.checkValidity();
         remove(it.m_index);
     }
 
-    template<typename T, size_t inlineCapacity>
-    inline void Deque<T, inlineCapacity>::remove(size_t position)
+    template<typename T>
+    inline void Deque<T>::remove(size_t position)
     {
         if (position == m_end)
             return;
@@ -496,28 +496,28 @@
     }
 
 #ifdef NDEBUG
-    template<typename T, size_t inlineCapacity> inline void DequeIteratorBase<T, inlineCapacity>::checkValidity() const { }
-    template<typename T, size_t inlineCapacity> inline void DequeIteratorBase<T, inlineCapacity>::checkValidity(const DequeIteratorBase<T, inlineCapacity>&) const { }
-    template<typename T, size_t inlineCapacity> inline void DequeIteratorBase<T, inlineCapacity>::addToIteratorsList() { }
-    template<typename T, size_t inlineCapacity> inline void DequeIteratorBase<T, inlineCapacity>::removeFromIteratorsList() { }
+    template<typename T> inline void DequeIteratorBase<T>::checkValidity() const { }
+    template<typename T> inline void DequeIteratorBase<T>::checkValidity(const DequeIteratorBase<T>&) const { }
+    template<typename T> inline void DequeIteratorBase<T>::addToIteratorsList() { }
+    template<typename T> inline void DequeIteratorBase<T>::removeFromIteratorsList() { }
 #else
-    template<typename T, size_t inlineCapacity>
-    void DequeIteratorBase<T, inlineCapacity>::checkValidity() const
+    template<typename T>
+    void DequeIteratorBase<T>::checkValidity() const
     {
         ASSERT(m_deque);
         m_deque->checkIndexValidity(m_index);
     }
 
-    template<typename T, size_t inlineCapacity>
-    void DequeIteratorBase<T, inlineCapacity>::checkValidity(const DequeIteratorBase& other) const
+    template<typename T>
+    void DequeIteratorBase<T>::checkValidity(const DequeIteratorBase& other) const
     {
         checkValidity();
         other.checkValidity();
         ASSERT(m_deque == other.m_deque);
     }
 
-    template<typename T, size_t inlineCapacity>
-    void DequeIteratorBase<T, inlineCapacity>::addToIteratorsList()
+    template<typename T>
+    void DequeIteratorBase<T>::addToIteratorsList()
     {
         if (!m_deque)
             m_next = 0;
@@ -530,8 +530,8 @@
         m_previous = 0;
     }
 
-    template<typename T, size_t inlineCapacity>
-    void DequeIteratorBase<T, inlineCapacity>::removeFromIteratorsList()
+    template<typename T>
+    void DequeIteratorBase<T>::removeFromIteratorsList()
     {
         if (!m_deque) {
             ASSERT(!m_next);
@@ -555,23 +555,23 @@
     }
 #endif
 
-    template<typename T, size_t inlineCapacity>
-    inline DequeIteratorBase<T, inlineCapacity>::DequeIteratorBase()
+    template<typename T>
+    inline DequeIteratorBase<T>::DequeIteratorBase()
         : m_deque(0)
     {
     }
 
-    template<typename T, size_t inlineCapacity>
-    inline DequeIteratorBase<T, inlineCapacity>::DequeIteratorBase(const Deque<T, inlineCapacity>* deque, size_t index)
-        : m_deque(const_cast<Deque<T, inlineCapacity>*>(deque))
+    template<typename T>
+    inline DequeIteratorBase<T>::DequeIteratorBase(const Deque<T>* deque, size_t index)
+        : m_deque(const_cast<Deque<T>*>(deque))
         , m_index(index)
     {
         addToIteratorsList();
         checkValidity();
     }
 
-    template<typename T, size_t inlineCapacity>
-    inline DequeIteratorBase<T, inlineCapacity>::DequeIteratorBase(const DequeIteratorBase& other)
+    template<typename T>
+    inline DequeIteratorBase<T>::DequeIteratorBase(const DequeIteratorBase& other)
         : m_deque(other.m_deque)
         , m_index(other.m_index)
     {
@@ -579,8 +579,8 @@
         checkValidity();
     }
 
-    template<typename T, size_t inlineCapacity>
-    inline DequeIteratorBase<T, inlineCapacity>& DequeIteratorBase<T, inlineCapacity>::operator=(const DequeIteratorBase& other)
+    template<typename T>
+    inline DequeIteratorBase<T>& DequeIteratorBase<T>::operator=(const DequeIteratorBase& other)
     {
         other.checkValidity();
         removeFromIteratorsList();
@@ -592,8 +592,8 @@
         return *this;
     }
 
-    template<typename T, size_t inlineCapacity>
-    inline DequeIteratorBase<T, inlineCapacity>::~DequeIteratorBase()
+    template<typename T>
+    inline DequeIteratorBase<T>::~DequeIteratorBase()
     {
 #ifndef NDEBUG
         removeFromIteratorsList();
@@ -601,15 +601,15 @@
 #endif
     }
 
-    template<typename T, size_t inlineCapacity>
-    inline bool DequeIteratorBase<T, inlineCapacity>::isEqual(const DequeIteratorBase& other) const
+    template<typename T>
+    inline bool DequeIteratorBase<T>::isEqual(const DequeIteratorBase& other) const
     {
         checkValidity(other);
         return m_index == other.m_index;
     }
 
-    template<typename T, size_t inlineCapacity>
-    inline void DequeIteratorBase<T, inlineCapacity>::increment()
+    template<typename T>
+    inline void DequeIteratorBase<T>::increment()
     {
         checkValidity();
         ASSERT(m_index != m_deque->m_end);
@@ -621,8 +621,8 @@
         checkValidity();
     }
 
-    template<typename T, size_t inlineCapacity>
-    inline void DequeIteratorBase<T, inlineCapacity>::decrement()
+    template<typename T>
+    inline void DequeIteratorBase<T>::decrement()
     {
         checkValidity();
         ASSERT(m_index != m_deque->m_start);
@@ -634,16 +634,16 @@
         checkValidity();
     }
 
-    template<typename T, size_t inlineCapacity>
-    inline T* DequeIteratorBase<T, inlineCapacity>::after() const
+    template<typename T>
+    inline T* DequeIteratorBase<T>::after() const
     {
         checkValidity();
         ASSERT(m_index != m_deque->m_end);
         return &m_deque->m_buffer.buffer()[m_index];
     }
 
-    template<typename T, size_t inlineCapacity>
-    inline T* DequeIteratorBase<T, inlineCapacity>::before() const
+    template<typename T>
+    inline T* DequeIteratorBase<T>::before() const
     {
         checkValidity();
         ASSERT(m_index != m_deque->m_start);

Modified: trunk/Source/WTF/wtf/Vector.h (164184 => 164185)


--- trunk/Source/WTF/wtf/Vector.h	2014-02-16 02:01:04 UTC (rev 164184)
+++ trunk/Source/WTF/wtf/Vector.h	2014-02-16 02:54:26 UTC (rev 164185)
@@ -374,7 +374,7 @@
         deallocateBuffer(buffer());
     }
     
-    void swap(VectorBuffer<T, 0>& other)
+    void swap(VectorBuffer<T, 0>& other, size_t, size_t)
     {
         std::swap(m_buffer, other.m_buffer);
         std::swap(m_capacity, other.m_capacity);
@@ -464,20 +464,20 @@
         Base::reallocateBuffer(newCapacity);
     }
 
-    void swap(VectorBuffer& other)
+    void swap(VectorBuffer& other, size_t mySize, size_t otherSize)
     {
         if (buffer() == inlineBuffer() && other.buffer() == other.inlineBuffer()) {
-            std::swap(m_inlineBuffer, other.m_inlineBuffer);
+            swapInlineBuffer(other, mySize, otherSize);
             std::swap(m_capacity, other.m_capacity);
         } else if (buffer() == inlineBuffer()) {
             m_buffer = other.m_buffer;
             other.m_buffer = other.inlineBuffer();
-            std::swap(m_inlineBuffer, other.m_inlineBuffer);
+            swapInlineBuffer(other, mySize, 0);
             std::swap(m_capacity, other.m_capacity);
         } else if (other.buffer() == other.inlineBuffer()) {
             other.m_buffer = m_buffer;
             m_buffer = inlineBuffer();
-            std::swap(m_inlineBuffer, other.m_inlineBuffer);
+            swapInlineBuffer(other, 0, otherSize);
             std::swap(m_capacity, other.m_capacity);
         } else {
             std::swap(m_buffer, other.m_buffer);
@@ -510,6 +510,32 @@
 private:
     using Base::m_buffer;
     using Base::m_capacity;
+    
+    void swapInlineBuffer(VectorBuffer& other, size_t mySize, size_t otherSize)
+    {
+        // FIXME: We could make swap part of VectorTypeOperations
+        // https://bugs.webkit.org/show_bug.cgi?id=128863
+        
+        if (std::is_pod<T>::value)
+            std::swap(m_inlineBuffer, other.m_inlineBuffer);
+        else
+            swapInlineBuffers(inlineBuffer(), other.inlineBuffer(), mySize, otherSize);
+    }
+    
+    static void swapInlineBuffers(T* left, T* right, size_t leftSize, size_t rightSize)
+    {
+        if (left == right)
+            return;
+        
+        ASSERT(leftSize <= inlineCapacity);
+        ASSERT(rightSize <= inlineCapacity);
+        
+        size_t swapBound = std::min(leftSize, rightSize);
+        for (unsigned i = 0; i < swapBound; ++i)
+            std::swap(left[i], right[i]);
+        VectorTypeOperations<T>::move(left + swapBound, left + leftSize, right + swapBound);
+        VectorTypeOperations<T>::move(right + swapBound, right + rightSize, left + swapBound);
+    }
 
     T* inlineBuffer() { return reinterpret_cast_ptr<T*>(m_inlineBuffer); }
     const T* inlineBuffer() const { return reinterpret_cast_ptr<const T*>(m_inlineBuffer); }
@@ -686,8 +712,8 @@
 
     void swap(Vector<T, inlineCapacity, OverflowHandler>& other)
     {
+        Base::swap(other, m_size, other.m_size);
         std::swap(m_size, other.m_size);
-        Base::swap(other);
     }
 
     void reverse();

Modified: trunk/Source/WebCore/ChangeLog (164184 => 164185)


--- trunk/Source/WebCore/ChangeLog	2014-02-16 02:01:04 UTC (rev 164184)
+++ trunk/Source/WebCore/ChangeLog	2014-02-16 02:54:26 UTC (rev 164185)
@@ -1,3 +1,17 @@
+2014-02-15  Filip Pizlo  <[email protected]>
+
+        Vector with inline capacity should work with non-PODs
+        https://bugs.webkit.org/show_bug.cgi?id=128864
+
+        Reviewed by Michael Saboff.
+
+        No new tests because no change behavior.
+        
+        Deques no longer have inline capacity because it was broken, and we didn't need it
+        here anyway.
+
+        * page/WheelEventDeltaTracker.h:
+
 2014-02-15  Andreas Kling  <[email protected]>
 
         Add checked casts for Event.

Modified: trunk/Source/WebCore/page/WheelEventDeltaTracker.h (164184 => 164185)


--- trunk/Source/WebCore/page/WheelEventDeltaTracker.h	2014-02-16 02:01:04 UTC (rev 164184)
+++ trunk/Source/WebCore/page/WheelEventDeltaTracker.h	2014-02-16 02:54:26 UTC (rev 164185)
@@ -56,7 +56,7 @@
     DominantScrollGestureDirection dominantScrollGestureDirection() const;
 
 private:
-    Deque<FloatSize, recentEventCount> m_recentWheelEventDeltas;
+    Deque<FloatSize> m_recentWheelEventDeltas;
     bool m_isTrackingDeltas;
 
 };

Modified: trunk/Tools/ChangeLog (164184 => 164185)


--- trunk/Tools/ChangeLog	2014-02-16 02:01:04 UTC (rev 164184)
+++ trunk/Tools/ChangeLog	2014-02-16 02:54:26 UTC (rev 164185)
@@ -1,3 +1,17 @@
+2014-02-15  Filip Pizlo  <[email protected]>
+
+        Vector with inline capacity should work with non-PODs
+        https://bugs.webkit.org/show_bug.cgi?id=128864
+
+        Reviewed by Michael Saboff.
+        
+        This test experiences really bizarre behavior on trunk without the rest of
+        this fix. On my machine, it usually times out because it gets itself into an
+        infinite loop of some kind. With the fix, it passes.
+
+        * TestWebKitAPI/Tests/WTF/Vector.cpp:
+        (TestWebKitAPI::TEST):
+
 2014-02-15  Ryuan Choi  <[email protected]>
 
         [EFL][WK1] Do not include cairo header in the public headers

Modified: trunk/Tools/TestWebKitAPI/Tests/WTF/Vector.cpp (164184 => 164185)


--- trunk/Tools/TestWebKitAPI/Tests/WTF/Vector.cpp	2014-02-16 02:01:04 UTC (rev 164184)
+++ trunk/Tools/TestWebKitAPI/Tests/WTF/Vector.cpp	2014-02-16 02:54:26 UTC (rev 164185)
@@ -216,4 +216,94 @@
     }
 }
 
+TEST(WTF_Vector, VectorOfVectorsOfVectorsInlineCapacitySwap)
+{
+    Vector<Vector<Vector<int, 1>, 1>, 1> a;
+    Vector<Vector<Vector<int, 1>, 1>, 1> b;
+    Vector<Vector<Vector<int, 1>, 1>, 1> c;
+    
+    EXPECT_EQ(a.size(), 0U);
+    EXPECT_EQ(b.size(), 0U);
+    EXPECT_EQ(c.size(), 0U);
+    
+    Vector<int, 1> x;
+    x.append(42);
+    
+    EXPECT_EQ(x.size(), 1U);
+    EXPECT_EQ(x[0], 42);
+    
+    Vector<Vector<int, 1>, 1> y;
+    y.append(x);
+    
+    EXPECT_EQ(x.size(), 1U);
+    EXPECT_EQ(x[0], 42);
+    EXPECT_EQ(y.size(), 1U);
+    EXPECT_EQ(y[0].size(), 1U);
+    EXPECT_EQ(y[0][0], 42);
+    
+    a.append(y);
+
+    EXPECT_EQ(x.size(), 1U);
+    EXPECT_EQ(x[0], 42);
+    EXPECT_EQ(y.size(), 1U);
+    EXPECT_EQ(y[0].size(), 1U);
+    EXPECT_EQ(y[0][0], 42);
+    EXPECT_EQ(a.size(), 1U);
+    EXPECT_EQ(a[0].size(), 1U);
+    EXPECT_EQ(a[0][0].size(), 1U);
+    EXPECT_EQ(a[0][0][0], 42);
+    
+    a.swap(b);
+
+    EXPECT_EQ(a.size(), 0U);
+    EXPECT_EQ(x.size(), 1U);
+    EXPECT_EQ(x[0], 42);
+    EXPECT_EQ(y.size(), 1U);
+    EXPECT_EQ(y[0].size(), 1U);
+    EXPECT_EQ(y[0][0], 42);
+    EXPECT_EQ(b.size(), 1U);
+    EXPECT_EQ(b[0].size(), 1U);
+    EXPECT_EQ(b[0][0].size(), 1U);
+    EXPECT_EQ(b[0][0][0], 42);
+    
+    b.swap(c);
+
+    EXPECT_EQ(a.size(), 0U);
+    EXPECT_EQ(b.size(), 0U);
+    EXPECT_EQ(x.size(), 1U);
+    EXPECT_EQ(x[0], 42);
+    EXPECT_EQ(y.size(), 1U);
+    EXPECT_EQ(y[0].size(), 1U);
+    EXPECT_EQ(y[0][0], 42);
+    EXPECT_EQ(c.size(), 1U);
+    EXPECT_EQ(c[0].size(), 1U);
+    EXPECT_EQ(c[0][0].size(), 1U);
+    EXPECT_EQ(c[0][0][0], 42);
+    
+    y[0][0] = 24;
+
+    EXPECT_EQ(x.size(), 1U);
+    EXPECT_EQ(x[0], 42);
+    EXPECT_EQ(y.size(), 1U);
+    EXPECT_EQ(y[0].size(), 1U);
+    EXPECT_EQ(y[0][0], 24);
+    
+    a.append(y);
+
+    EXPECT_EQ(x.size(), 1U);
+    EXPECT_EQ(x[0], 42);
+    EXPECT_EQ(y.size(), 1U);
+    EXPECT_EQ(y[0].size(), 1U);
+    EXPECT_EQ(y[0][0], 24);
+    EXPECT_EQ(a.size(), 1U);
+    EXPECT_EQ(a[0].size(), 1U);
+    EXPECT_EQ(a[0][0].size(), 1U);
+    EXPECT_EQ(a[0][0][0], 24);
+    EXPECT_EQ(c.size(), 1U);
+    EXPECT_EQ(c[0].size(), 1U);
+    EXPECT_EQ(c[0][0].size(), 1U);
+    EXPECT_EQ(c[0][0][0], 42);
+    EXPECT_EQ(b.size(), 0U);
+}
+
 } // namespace TestWebKitAPI

_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to