EPUB parsing optimizations

pkg/archive/archive_zip.go

@@ -320,8 +320,45 @@ func (e *gozipArchiveEntry) ReadCompressedGzip() ([]byte, error) {
 type gozipArchive struct {
 	zip           *zip.Reader
 	closer        func() error
-	cachedEntries sync.Map
 	minimizeReads bool
+
+	// nameIndex maps each entry's cleaned path to its *zip.File, giving O(1)
+	// lookups instead of a linear scan. It's built once, lazily, on the first
+	// Entry call that misses the wrapper cache.
+	indexOnce sync.Once
+	nameIndex map[string]*zip.File
+
+	// cachedEntries holds lazily-created *gozipArchiveEntry wrappers, keyed by
+	// cleaned path. Wrappers are shared so per-entry read state (e.g. the
+	// deflate random-access index) persists across reads of the same entry.
+	cachedEntries sync.Map
+}
+
+// buildIndex populates nameIndex from the archive's central directory. First
+// occurrence of a cleaned path wins, matching the previous linear-scan lookup.
+func (a *gozipArchive) buildIndex() {
+	nameIndex := make(map[string]*zip.File, len(a.zip.File))
+	for _, f := range a.zip.File {
+		cp := path.Clean(f.Name)
+		if _, ok := nameIndex[cp]; !ok {
+			nameIndex[cp] = f
+		}
+	}
+	a.nameIndex = nameIndex
+}
+
+// wrap returns the shared entry wrapper for f, creating it on first access.
+// LoadOrStore guarantees a single wrapper per entry even under concurrent Get.
+func (a *gozipArchive) wrap(cleanPath string, f *zip.File) *gozipArchiveEntry {
+	if e, ok := a.cachedEntries.Load(cleanPath); ok {
+		return e.(*gozipArchiveEntry)
+	}
+	entry := &gozipArchiveEntry{
+		file:          f,
+		minimizeReads: a.minimizeReads,
+	}
+	actual, _ := a.cachedEntries.LoadOrStore(cleanPath, entry)
+	return actual.(*gozipArchiveEntry)
 }
 
 // Close implements Archive
@@ -336,16 +373,7 @@ func (a *gozipArchive) Entries() []Entry {
 		if f.FileInfo().IsDir() {
 			continue
 		}
-
-		aentry, ok := a.cachedEntries.Load(f.Name)
-		if !ok {
-			aentry = &gozipArchiveEntry{
-				file:          f,
-				minimizeReads: a.minimizeReads,
-			}
-			a.cachedEntries.Store(f.Name, aentry)
-		}
-		entries = append(entries, aentry.(Entry))
+		entries = append(entries, a.wrap(path.Clean(f.Name), f))
 	}
 	return entries
 }
@@ -357,24 +385,17 @@ func (a *gozipArchive) Entry(p string) (Entry, error) {
 	}
 	cpath := path.Clean(p)
 
-	// Check for entry in cache
-	aentry, ok := a.cachedEntries.Load(cpath)
-	if ok { // Found entry in cache
+	// Check for an already-wrapped entry first.
+	if aentry, ok := a.cachedEntries.Load(cpath); ok {
 		return aentry.(Entry), nil
 	}
 
-	for _, f := range a.zip.File {
-		fp := path.Clean(f.Name)
-		if fp == cpath {
-			aentry := &gozipArchiveEntry{
-				file:          f,
-				minimizeReads: a.minimizeReads,
-			}
-			a.cachedEntries.Store(fp, aentry) // Put entry in cache
-			return aentry, nil
-		}
+	a.indexOnce.Do(a.buildIndex)
+	f, ok := a.nameIndex[cpath]
+	if !ok {
+		return nil, fs.ErrNotExist
 	}
-	return nil, fs.ErrNotExist
+	return a.wrap(cpath, f), nil
 }
 
 func NewGoZIPArchive(zip *zip.Reader, closer func() error, minimizeReads bool) Archive {

pkg/parser/epub/parser.go

@@ -3,6 +3,7 @@ package epub
 import (
 	"context"
 
+	"github.com/antchfx/xmlquery"
 	"github.com/pkg/errors"
 	"github.com/readium/go-toolkit/pkg/asset"
 	"github.com/readium/go-toolkit/pkg/content/iterator"
@@ -57,12 +58,12 @@ func (p Parser) Parse(ctx context.Context, asset asset.PublicationAsset, f fetch
 	// themselves through other well-known files (rights.xml / sinf.xml).
 	// TODO: surface the publication-level scheme on the manifest itself so
 	// consumers can detect protection even when encryption.xml is absent.
-	scheme, err := protection.IdentifyEPUBProtection(ctx, f)
+	scheme, encryptionDoc, err := protection.IdentifyEPUBProtection(ctx, f)
 	if err != nil {
 		return nil, errors.Wrap(err, "failed identifying EPUB protection scheme")
 	}
 
-	encryptionData, err := parseEncryptionData(ctx, f, scheme.URI())
+	encryptionData, err := parseEncryptionData(ctx, f, scheme.URI(), encryptionDoc)
 	if err != nil {
 		return nil, errors.Wrap(err, "failed parsing encryption data")
 	}
@@ -94,12 +95,19 @@ func (p Parser) Parse(ctx context.Context, asset asset.PublicationAsset, f fetch
 // each entry with the supplied DRM scheme URI (typically obtained by calling
 // [protection.IdentifyEPUBProtection] at a higher level). A missing
 // encryption.xml is normal and returns (nil, nil).
-func parseEncryptionData(ctx context.Context, f fetcher.Fetcher, scheme string) (map[string]manifest.Encryption, error) {
-	n, rerr := fetcher.ReadResourceAsXML(ctx, f.Get(ctx, manifest.Link{Href: manifest.MustNewHREFFromString("META-INF/encryption.xml", false)}))
-	if rerr != nil {
-		return nil, nil
+//
+// When doc is non-nil it is used directly, avoiding a redundant read+parse of
+// encryption.xml — [protection.IdentifyEPUBProtection] returns the document it
+// already parsed for exactly this purpose.
+func parseEncryptionData(ctx context.Context, f fetcher.Fetcher, scheme string, doc *xmlquery.Node) (map[string]manifest.Encryption, error) {
+	if doc == nil {
+		var rerr *fetcher.ResourceError
+		doc, rerr = fetcher.ReadResourceAsXML(ctx, f.Get(ctx, manifest.Link{Href: manifest.MustNewHREFFromString("META-INF/encryption.xml", false)}))
+		if rerr != nil {
+			return nil, nil
+		}
 	}
-	return ParseEncryption(n, scheme), nil
+	return ParseEncryption(doc, scheme), nil
 }
 
 func parseNavigationData(ctx context.Context, packageDocument PackageDocument, f fetcher.Fetcher) (ret map[string]manifest.LinkList) {

pkg/parser/epub/parser_bench_test.go

@@ -0,0 +1,43 @@
+package epub
+
+import (
+	"context"
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/readium/go-toolkit/pkg/fetcher"
+	"github.com/stretchr/testify/require"
+)
+
+func BenchmarkParser(b *testing.B) {
+	pubsDir := "../../../publications"
+	files, err := os.ReadDir(pubsDir)
+	if err != nil {
+		b.Skip("publications directory not found")
+	}
+
+	for _, file := range files {
+		if file.IsDir() || filepath.Ext(file.Name()) != ".epub" {
+			continue
+		}
+
+		b.Run(file.Name(), func(b *testing.B) {
+			path := filepath.Join(pubsDir, file.Name())
+
+			parser := NewParser(nil)
+			asset := fakeEPUBAsset{name: file.Name()}
+			for i := 0; i < b.N; i++ {
+				// We recreate fetcher in each iteration because in real world we parse from fresh fetcher.
+				f, err := fetcher.NewArchiveFetcherFromPath(context.Background(), path)
+				require.NoError(b, err)
+				func() {
+					defer f.Close()
+					builder, err := parser.Parse(context.Background(), asset, f)
+					require.NoError(b, err)
+					require.NotNil(b, builder)
+				}()
+			}
+		})
+	}
+}

pkg/parser/epub/parser_navdoc.go

@@ -92,21 +92,26 @@ func parseLiElement(li *xmlquery.Node, filePath url.URL) (link *manifest.Link) {
 	}
 	var title string
 	if first.Data != "ol" {
-		title = strings.TrimSpace(muchSpaceSuchWowMatcher.ReplaceAllString(first.InnerText(), " "))
+		title = collapseWhitespace(first.InnerText())
 	}
 	rawHref := first.SelectAttr("href")
-	href := url.MustURLFromString("#")
+	var href url.URL
 	if first.Data == "a" && rawHref != "" {
-		s, err := url.FromEPUBHref(rawHref)
-		if err == nil {
+		if s, err := url.FromEPUBHref(rawHref); err == nil {
 			href = filePath.Resolve(s)
 		}
 	}
 
 	children := parseOlElement(xmlquery.QuerySelector(li, xpNavOL), filePath)
-	if len(children) == 0 && (href.String() == "" || title == "") {
+	// A nil href here is the lazy stand-in for the old "#" default, whose
+	// String() is "" — so a missing or empty-resolved href drops the entry
+	// (unless it has children), exactly as before.
+	if len(children) == 0 && (href == nil || href.String() == "" || title == "") {
 		return nil
 	}
+	if href == nil {
+		href = url.MustURLFromString("#")
+	}
 	return &manifest.Link{
 		Title:    title,
 		Href:     manifest.NewHREF(href),

pkg/parser/epub/parser_ncx.go

@@ -1,8 +1,6 @@
 package epub
 
 import (
-	"strings"
-
 	"github.com/antchfx/xmlquery"
 	"github.com/readium/go-toolkit/pkg/manifest"
 	"github.com/readium/go-toolkit/pkg/util/url"
@@ -92,7 +90,7 @@ func extractTitle(element *xmlquery.Node) string {
 	if tel == nil {
 		return ""
 	}
-	return strings.TrimSpace(muchSpaceSuchWowMatcher.ReplaceAllString(tel.InnerText(), " "))
+	return collapseWhitespace(tel.InnerText())
 }
 
 func extractHref(element *xmlquery.Node, filePath url.URL) url.URL {

pkg/parser/epub/parser_packagedoc.go

@@ -178,7 +178,7 @@ func ParseItemRef(element *xmlquery.Node, prefixMap map[string]string) *ItemRef
 
 	pp := parseProperties(element.SelectAttr("properties"))
 	properties := make([]string, 0, len(pp))
-	for _, prop := range parseProperties(element.SelectAttr("properties")) {
+	for _, prop := range pp {
 		if prop == "" {
 			continue
 		}

pkg/parser/epub/parser_test.go

@@ -107,10 +107,10 @@ func TestParseEncryptionDataScheme(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			f := openProtectionFixture(t, tt.file)
 
-			scheme, err := protection.IdentifyEPUBProtection(t.Context(), f)
+			scheme, encryptionDoc, err := protection.IdentifyEPUBProtection(t.Context(), f)
 			require.NoError(t, err)
 
-			enc, err := parseEncryptionData(t.Context(), f, scheme.URI())
+			enc, err := parseEncryptionData(t.Context(), f, scheme.URI(), encryptionDoc)
 			require.NoError(t, err)
 			if !tt.expectEntries {
 				assert.Empty(t, enc)

pkg/parser/epub/property_data_type.go

@@ -78,15 +78,34 @@ func parsePrefixes(prefixes string) map[string]string {
 	return p
 }
 
-var muchSpaceSuchWowMatcher = regexp.MustCompile(`\s+`)
-
 func parseProperties(raw string) []string {
-	vals := muchSpaceSuchWowMatcher.Split(raw, -1)
-	s := make([]string, 0, len(vals))
-	for _, v := range vals {
-		if v != "" {
-			s = append(s, v)
+	return strings.Fields(raw)
+}
+
+// collapseWhitespace trims leading/trailing ASCII whitespace and replaces every
+// internal run of it with a single space. It reproduces the previous
+// regexp.ReplaceAllString(`\s+`, " ") + TrimSpace behavior — deliberately ASCII
+// only (the same set RE2's \s matches), so non-ASCII spaces such as the U+3000
+// ideographic space common in CJK titles are preserved. Scanning byte-wise is
+// safe because UTF-8 continuation bytes are always >= 0x80 and never collide
+// with ASCII whitespace.
+func collapseWhitespace(s string) string {
+	var b strings.Builder
+	b.Grow(len(s))
+	pendingSpace := false
+	for i := 0; i < len(s); i++ {
+		switch s[i] {
+		case ' ', '\t', '\n', '\f', '\r':
+			if b.Len() > 0 {
+				pendingSpace = true
+			}
+		default:
+			if pendingSpace {
+				b.WriteByte(' ')
+				pendingSpace = false
+			}
+			b.WriteByte(s[i])
 		}
 	}
-	return s
+	return b.String()
 }