Переглянути джерело

refactor(opml): reduce indirections

Don't use a slice of pointers to opml items, when we can simply use a slice of
items instead. This should reduce the amount of memory allocations and the
number of indirections the GC has to process, speedup up the import process.

Note that this doesn't introduce any additional copies, as the only time a
slice of subscription is created, the items are created and inserted inline.
jvoisin 8 місяців тому
батько
коміт
93fc206f42

+ 2 - 2
internal/reader/opml/handler.go

@@ -23,9 +23,9 @@ func (h *Handler) Export(userID int64) (string, error) {
 		return "", err
 		return "", err
 	}
 	}
 
 
-	subscriptions := make(subcriptionList, 0, len(feeds))
+	subscriptions := make([]subcription, 0, len(feeds))
 	for _, feed := range feeds {
 	for _, feed := range feeds {
-		subscriptions = append(subscriptions, &subcription{
+		subscriptions = append(subscriptions, subcription{
 			Title:        feed.Title,
 			Title:        feed.Title,
 			FeedURL:      feed.FeedURL,
 			FeedURL:      feed.FeedURL,
 			SiteURL:      feed.SiteURL,
 			SiteURL:      feed.SiteURL,

+ 6 - 4
internal/reader/opml/parser.go

@@ -11,8 +11,8 @@ import (
 	"miniflux.app/v2/internal/reader/encoding"
 	"miniflux.app/v2/internal/reader/encoding"
 )
 )
 
 
-// parse reads an OPML file and returns a SubcriptionList.
-func parse(data io.Reader) (subcriptionList, error) {
+// parse reads an OPML file and returns a list of subscription.
+func parse(data io.Reader) ([]subcription, error) {
 	opmlDocument := &opmlDocument{}
 	opmlDocument := &opmlDocument{}
 	decoder := xml.NewDecoder(data)
 	decoder := xml.NewDecoder(data)
 	decoder.Entity = xml.HTMLEntity
 	decoder.Entity = xml.HTMLEntity
@@ -27,10 +27,12 @@ func parse(data io.Reader) (subcriptionList, error) {
 	return getSubscriptionsFromOutlines(opmlDocument.Outlines, ""), nil
 	return getSubscriptionsFromOutlines(opmlDocument.Outlines, ""), nil
 }
 }
 
 
-func getSubscriptionsFromOutlines(outlines opmlOutlineCollection, category string) (subscriptions subcriptionList) {
+func getSubscriptionsFromOutlines(outlines opmlOutlineCollection, category string) []subcription {
+	subscriptions := make([]subcription, 0, len(outlines))
+
 	for _, outline := range outlines {
 	for _, outline := range outlines {
 		if outline.IsSubscription() {
 		if outline.IsSubscription() {
-			subscriptions = append(subscriptions, &subcription{
+			subscriptions = append(subscriptions, subcription{
 				Title:        outline.GetTitle(),
 				Title:        outline.GetTitle(),
 				FeedURL:      outline.FeedURL,
 				FeedURL:      outline.FeedURL,
 				SiteURL:      outline.GetSiteURL(),
 				SiteURL:      outline.GetSiteURL(),

+ 22 - 22
internal/reader/opml/parser_test.go

@@ -9,7 +9,7 @@ import (
 )
 )
 
 
 // equals compare two subscriptions.
 // equals compare two subscriptions.
-func (s subcription) equals(subscription *subcription) bool {
+func (s subcription) equals(subscription subcription) bool {
 	return s.Title == subscription.Title && s.SiteURL == subscription.SiteURL &&
 	return s.Title == subscription.Title && s.SiteURL == subscription.SiteURL &&
 		s.FeedURL == subscription.FeedURL && s.CategoryName == subscription.CategoryName &&
 		s.FeedURL == subscription.FeedURL && s.CategoryName == subscription.CategoryName &&
 		s.Description == subscription.Description
 		s.Description == subscription.Description
@@ -39,8 +39,8 @@ func TestParseOpmlWithoutCategories(t *testing.T) {
 	</opml>
 	</opml>
 	`
 	`
 
 
-	var expected subcriptionList
-	expected = append(expected, &subcription{Title: "CNET News.com", FeedURL: "http://news.com.com/2547-1_3-0-5.xml", SiteURL: "http://news.com.com/", Description: "Tech news and business reports by CNET News.com. Focused on information technology, core topics include computers, hardware, software, networking, and Internet media."})
+	var expected []subcription
+	expected = append(expected, subcription{Title: "CNET News.com", FeedURL: "http://news.com.com/2547-1_3-0-5.xml", SiteURL: "http://news.com.com/", Description: "Tech news and business reports by CNET News.com. Focused on information technology, core topics include computers, hardware, software, networking, and Internet media."})
 
 
 	subscriptions, err := parse(bytes.NewBufferString(data))
 	subscriptions, err := parse(bytes.NewBufferString(data))
 	if err != nil {
 	if err != nil {
@@ -74,10 +74,10 @@ func TestParseOpmlWithCategories(t *testing.T) {
 	</opml>
 	</opml>
 	`
 	`
 
 
-	var expected subcriptionList
-	expected = append(expected, &subcription{Title: "Feed 1", FeedURL: "http://example.org/feed1/", SiteURL: "http://example.org/1", CategoryName: "My Category 1"})
-	expected = append(expected, &subcription{Title: "Feed 2", FeedURL: "http://example.org/feed2/", SiteURL: "http://example.org/2", CategoryName: "My Category 1"})
-	expected = append(expected, &subcription{Title: "Feed 3", FeedURL: "http://example.org/feed3/", SiteURL: "http://example.org/3", CategoryName: "My Category 2"})
+	var expected []subcription
+	expected = append(expected, subcription{Title: "Feed 1", FeedURL: "http://example.org/feed1/", SiteURL: "http://example.org/1", CategoryName: "My Category 1"})
+	expected = append(expected, subcription{Title: "Feed 2", FeedURL: "http://example.org/feed2/", SiteURL: "http://example.org/2", CategoryName: "My Category 1"})
+	expected = append(expected, subcription{Title: "Feed 3", FeedURL: "http://example.org/feed3/", SiteURL: "http://example.org/3", CategoryName: "My Category 2"})
 
 
 	subscriptions, err := parse(bytes.NewBufferString(data))
 	subscriptions, err := parse(bytes.NewBufferString(data))
 	if err != nil {
 	if err != nil {
@@ -108,9 +108,9 @@ func TestParseOpmlWithEmptyTitleAndEmptySiteURL(t *testing.T) {
 	</opml>
 	</opml>
 	`
 	`
 
 
-	var expected subcriptionList
-	expected = append(expected, &subcription{Title: "http://example.org/1", FeedURL: "http://example.org/feed1/", SiteURL: "http://example.org/1", CategoryName: ""})
-	expected = append(expected, &subcription{Title: "http://example.org/feed2/", FeedURL: "http://example.org/feed2/", SiteURL: "http://example.org/feed2/", CategoryName: ""})
+	var expected []subcription
+	expected = append(expected, subcription{Title: "http://example.org/1", FeedURL: "http://example.org/feed1/", SiteURL: "http://example.org/1", CategoryName: ""})
+	expected = append(expected, subcription{Title: "http://example.org/feed2/", FeedURL: "http://example.org/feed2/", SiteURL: "http://example.org/feed2/", CategoryName: ""})
 
 
 	subscriptions, err := parse(bytes.NewBufferString(data))
 	subscriptions, err := parse(bytes.NewBufferString(data))
 	if err != nil {
 	if err != nil {
@@ -146,9 +146,9 @@ func TestParseOpmlVersion1(t *testing.T) {
 	</opml>
 	</opml>
 	`
 	`
 
 
-	var expected subcriptionList
-	expected = append(expected, &subcription{Title: "Feed 1", FeedURL: "http://example.org/feed1/", SiteURL: "http://example.org/1", CategoryName: "Category 1"})
-	expected = append(expected, &subcription{Title: "Feed 2", FeedURL: "http://example.org/feed2/", SiteURL: "http://example.org/2", CategoryName: "Category 2"})
+	var expected []subcription
+	expected = append(expected, subcription{Title: "Feed 1", FeedURL: "http://example.org/feed1/", SiteURL: "http://example.org/1", CategoryName: "Category 1"})
+	expected = append(expected, subcription{Title: "Feed 2", FeedURL: "http://example.org/feed2/", SiteURL: "http://example.org/2", CategoryName: "Category 2"})
 
 
 	subscriptions, err := parse(bytes.NewBufferString(data))
 	subscriptions, err := parse(bytes.NewBufferString(data))
 	if err != nil {
 	if err != nil {
@@ -180,9 +180,9 @@ func TestParseOpmlVersion1WithoutOuterOutline(t *testing.T) {
 	</opml>
 	</opml>
 	`
 	`
 
 
-	var expected subcriptionList
-	expected = append(expected, &subcription{Title: "Feed 1", FeedURL: "http://example.org/feed1/", SiteURL: "http://example.org/1", CategoryName: ""})
-	expected = append(expected, &subcription{Title: "Feed 2", FeedURL: "http://example.org/feed2/", SiteURL: "http://example.org/2", CategoryName: ""})
+	var expected []subcription
+	expected = append(expected, subcription{Title: "Feed 1", FeedURL: "http://example.org/feed1/", SiteURL: "http://example.org/1", CategoryName: ""})
+	expected = append(expected, subcription{Title: "Feed 2", FeedURL: "http://example.org/feed2/", SiteURL: "http://example.org/2", CategoryName: ""})
 
 
 	subscriptions, err := parse(bytes.NewBufferString(data))
 	subscriptions, err := parse(bytes.NewBufferString(data))
 	if err != nil {
 	if err != nil {
@@ -221,10 +221,10 @@ func TestParseOpmlVersion1WithSeveralNestedOutlines(t *testing.T) {
 	</opml>
 	</opml>
 	`
 	`
 
 
-	var expected subcriptionList
-	expected = append(expected, &subcription{Title: "Feed 1", FeedURL: "http://example.org/feed1/", SiteURL: "http://example.org/1", CategoryName: "Some Category"})
-	expected = append(expected, &subcription{Title: "Feed 2", FeedURL: "http://example.org/feed2/", SiteURL: "http://example.org/2", CategoryName: "Some Category"})
-	expected = append(expected, &subcription{Title: "Feed 3", FeedURL: "http://example.org/feed3/", SiteURL: "http://example.org/3", CategoryName: "Another Category"})
+	var expected []subcription
+	expected = append(expected, subcription{Title: "Feed 1", FeedURL: "http://example.org/feed1/", SiteURL: "http://example.org/1", CategoryName: "Some Category"})
+	expected = append(expected, subcription{Title: "Feed 2", FeedURL: "http://example.org/feed2/", SiteURL: "http://example.org/2", CategoryName: "Some Category"})
+	expected = append(expected, subcription{Title: "Feed 3", FeedURL: "http://example.org/feed3/", SiteURL: "http://example.org/3", CategoryName: "Another Category"})
 
 
 	subscriptions, err := parse(bytes.NewBufferString(data))
 	subscriptions, err := parse(bytes.NewBufferString(data))
 	if err != nil {
 	if err != nil {
@@ -256,8 +256,8 @@ func TestParseOpmlWithInvalidCharacterEntity(t *testing.T) {
 	</opml>
 	</opml>
 	`
 	`
 
 
-	var expected subcriptionList
-	expected = append(expected, &subcription{Title: "Feed 1", FeedURL: "http://example.org/feed1/a&b", SiteURL: "http://example.org/c&d", CategoryName: "Feed 1"})
+	var expected []subcription
+	expected = append(expected, subcription{Title: "Feed 1", FeedURL: "http://example.org/feed1/a&b", SiteURL: "http://example.org/c&d", CategoryName: "Feed 1"})
 
 
 	subscriptions, err := parse(bytes.NewBufferString(data))
 	subscriptions, err := parse(bytes.NewBufferString(data))
 	if err != nil {
 	if err != nil {

+ 4 - 4
internal/reader/opml/serializer.go

@@ -13,7 +13,7 @@ import (
 )
 )
 
 
 // serialize returns a SubcriptionList in OPML format.
 // serialize returns a SubcriptionList in OPML format.
-func serialize(subscriptions subcriptionList) string {
+func serialize(subscriptions []subcription) string {
 	var b bytes.Buffer
 	var b bytes.Buffer
 	writer := bufio.NewWriter(&b)
 	writer := bufio.NewWriter(&b)
 	writer.WriteString(xml.Header)
 	writer.WriteString(xml.Header)
@@ -31,7 +31,7 @@ func serialize(subscriptions subcriptionList) string {
 	return b.String()
 	return b.String()
 }
 }
 
 
-func convertSubscriptionsToOPML(subscriptions subcriptionList) *opmlDocument {
+func convertSubscriptionsToOPML(subscriptions []subcription) *opmlDocument {
 	opmlDocument := &opmlDocument{}
 	opmlDocument := &opmlDocument{}
 	opmlDocument.Version = "2.0"
 	opmlDocument.Version = "2.0"
 	opmlDocument.Header.Title = "Miniflux"
 	opmlDocument.Header.Title = "Miniflux"
@@ -62,8 +62,8 @@ func convertSubscriptionsToOPML(subscriptions subcriptionList) *opmlDocument {
 	return opmlDocument
 	return opmlDocument
 }
 }
 
 
-func groupSubscriptionsByFeed(subscriptions subcriptionList) map[string]subcriptionList {
-	groups := make(map[string]subcriptionList)
+func groupSubscriptionsByFeed(subscriptions []subcription) map[string][]subcription {
+	groups := make(map[string][]subcription)
 
 
 	for _, subscription := range subscriptions {
 	for _, subscription := range subscriptions {
 		groups[subscription.CategoryName] = append(groups[subscription.CategoryName], subscription)
 		groups[subscription.CategoryName] = append(groups[subscription.CategoryName], subscription)

+ 8 - 8
internal/reader/opml/serializer_test.go

@@ -9,10 +9,10 @@ import (
 )
 )
 
 
 func TestSerialize(t *testing.T) {
 func TestSerialize(t *testing.T) {
-	var subscriptions subcriptionList
-	subscriptions = append(subscriptions, &subcription{Title: "Feed 1", FeedURL: "http://example.org/feed/1", SiteURL: "http://example.org/1", CategoryName: "Category 1"})
-	subscriptions = append(subscriptions, &subcription{Title: "Feed 2", FeedURL: "http://example.org/feed/2", SiteURL: "http://example.org/2", CategoryName: "Category 1"})
-	subscriptions = append(subscriptions, &subcription{Title: "Feed 3", FeedURL: "http://example.org/feed/3", SiteURL: "http://example.org/3", CategoryName: "Category 2"})
+	var subscriptions []subcription
+	subscriptions = append(subscriptions, subcription{Title: "Feed 1", FeedURL: "http://example.org/feed/1", SiteURL: "http://example.org/1", CategoryName: "Category 1"})
+	subscriptions = append(subscriptions, subcription{Title: "Feed 2", FeedURL: "http://example.org/feed/2", SiteURL: "http://example.org/2", CategoryName: "Category 1"})
+	subscriptions = append(subscriptions, subcription{Title: "Feed 3", FeedURL: "http://example.org/feed/3", SiteURL: "http://example.org/3", CategoryName: "Category 2"})
 
 
 	output := serialize(subscriptions)
 	output := serialize(subscriptions)
 	feeds, err := parse(bytes.NewBufferString(output))
 	feeds, err := parse(bytes.NewBufferString(output))
@@ -48,10 +48,10 @@ func TestNormalizedCategoriesOrder(t *testing.T) {
 		{"Category 1", "Category 3"},
 		{"Category 1", "Category 3"},
 	}
 	}
 
 
-	var subscriptions subcriptionList
-	subscriptions = append(subscriptions, &subcription{Title: "Feed 1", FeedURL: "http://example.org/feed/1", SiteURL: "http://example.org/1", CategoryName: orderTests[0].naturalOrderName})
-	subscriptions = append(subscriptions, &subcription{Title: "Feed 2", FeedURL: "http://example.org/feed/2", SiteURL: "http://example.org/2", CategoryName: orderTests[1].naturalOrderName})
-	subscriptions = append(subscriptions, &subcription{Title: "Feed 3", FeedURL: "http://example.org/feed/3", SiteURL: "http://example.org/3", CategoryName: orderTests[2].naturalOrderName})
+	var subscriptions []subcription
+	subscriptions = append(subscriptions, subcription{Title: "Feed 1", FeedURL: "http://example.org/feed/1", SiteURL: "http://example.org/1", CategoryName: orderTests[0].naturalOrderName})
+	subscriptions = append(subscriptions, subcription{Title: "Feed 2", FeedURL: "http://example.org/feed/2", SiteURL: "http://example.org/2", CategoryName: orderTests[1].naturalOrderName})
+	subscriptions = append(subscriptions, subcription{Title: "Feed 3", FeedURL: "http://example.org/feed/3", SiteURL: "http://example.org/3", CategoryName: orderTests[2].naturalOrderName})
 
 
 	feeds := convertSubscriptionsToOPML(subscriptions)
 	feeds := convertSubscriptionsToOPML(subscriptions)
 
 

+ 0 - 3
internal/reader/opml/subscription.go

@@ -11,6 +11,3 @@ type subcription struct {
 	CategoryName string
 	CategoryName string
 	Description  string
 	Description  string
 }
 }
-
-// subcriptionList is a list of subscriptions.
-type subcriptionList []*subcription