From 0a710eeb9da7e2fb9afe3c3c9784b37389fc705f Mon Sep 17 00:00:00 2001 From: liu Date: Mon, 18 Oct 2021 19:31:33 +0800 Subject: [PATCH] fix: make it sorted when marshalling integer key map (#117) --- encoder/mapiter.go | 2 +- encoder/sort.go | 34 ++++++++------ issue_test/issue115_test.go | 90 +++++++++++++++++++++++++++++++++++++ issue_test/issue8_test.go | 7 +-- issue_test/issue90_test.go | 6 +-- 5 files changed, 118 insertions(+), 21 deletions(-) create mode 100644 issue_test/issue115_test.go diff --git a/encoder/mapiter.go b/encoder/mapiter.go index 583964d..d96ead7 100644 --- a/encoder/mapiter.go +++ b/encoder/mapiter.go @@ -27,7 +27,7 @@ import ( ) type _MapPair struct { - k string + k string // when the map key is integer, k is pointed to m v unsafe.Pointer m [32]byte } diff --git a/encoder/sort.go b/encoder/sort.go index 5d3f6a1..b1a6759 100644 --- a/encoder/sort.go +++ b/encoder/sort.go @@ -19,12 +19,16 @@ package encoder // Algorithm 3-way Radix Quicksort, d means the radix. // Reference: https://algs4.cs.princeton.edu/51radix/Quick3string.java.html func radixQsort(kvs []_MapPair, d, maxDepth int) { - if maxDepth == 0 { - heapSort(kvs, 0, len(kvs)) - return - } - maxDepth-- for len(kvs) > 11 { + // To avoid the worst case of quickSort (time: O(n^2)), use introsort here. + // Reference: https://en.wikipedia.org/wiki/Introsort and + // https://github.com/golang/go/issues/467 + if maxDepth == 0 { + heapSort(kvs, 0, len(kvs)) + return + } + maxDepth-- + p := pivot(kvs, d) lt, i, gt := 0, 0, len(kvs) for i < gt { @@ -42,13 +46,13 @@ func radixQsort(kvs []_MapPair, d, maxDepth int) { } // kvs[0:lt] < v = kvs[lt:gt] < kvs[gt:len(kvs)] - // Native: - // radixQsort(kvs[:lt], d, maxDepth) - // if p > -1 { - // radixQsort(kvs[lt:gt], d+1, maxDepth) - // } - // radixQsort(kvs[gt:], d, maxDepth) - // Optimize: make recursive calls only for the smaller parts. + // Native implemention: + // radixQsort(kvs[:lt], d, maxDepth) + // if p > -1 { + // radixQsort(kvs[lt:gt], d+1, maxDepth) + // } + // radixQsort(kvs[gt:], d, maxDepth) + // Optimize as follows: make recursive calls only for the smaller parts. // Reference: https://www.geeksforgeeks.org/quicksort-tail-call-optimization-reducing-worst-case-space-log-n/ if p == -1 { if lt > len(kvs) - gt { @@ -82,7 +86,7 @@ func radixQsort(kvs []_MapPair, d, maxDepth int) { func insertRadixSort(kvs []_MapPair, d int) { for i := 1; i < len(kvs); i++ { for j := i; j > 0 && lessFrom(kvs[j].k, kvs[j-1].k, d); j-- { - kvs[j], kvs[j-1] = kvs[j-1], kvs[j] + swap(kvs, j, j-1) } } } @@ -173,8 +177,10 @@ func heapSort(kvs []_MapPair, a, b int) { } } +// Note that _MapPair.k is NOT pointed to _MapPair.m when map key is integer after swap func swap(kvs []_MapPair, a, b int) { - kvs[a], kvs[b] = kvs[b], kvs[a] + kvs[a].k, kvs[b].k = kvs[b].k, kvs[a].k + kvs[a].v, kvs[b].v = kvs[b].v, kvs[a].v } // Compare two strings from the pos d. diff --git a/issue_test/issue115_test.go b/issue_test/issue115_test.go new file mode 100644 index 0000000..94094fe --- /dev/null +++ b/issue_test/issue115_test.go @@ -0,0 +1,90 @@ +/* + * Copyright 2021 ByteDance Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + + package issue_test + + import ( + `encoding` + `encoding/json` + `strconv` + `testing` + + `github.com/bytedance/sonic/encoder` + `github.com/stretchr/testify/require` +) + +type ptrTextMarshaler string + +func (k *ptrTextMarshaler) MarshalText() ([]byte, error) { + if k == nil { + return []byte("ptrisnil"), nil + } + return []byte("ptrto" + string(*k)), nil +} + +type textMarshaler string + +func (k textMarshaler) MarshalText() ([]byte, error) { + return []byte(string(k)), nil +} + +func TestIssue115_MarshalMapWithSort(t *testing.T) { + nptext := (*ptrTextMarshaler)(nil) + ptext := ptrTextMarshaler("key") + text0 := textMarshaler("") + text1 := textMarshaler("1") + text2 := textMarshaler("2") + testCases := []struct { + v interface{} + want string + }{ + { v: map[string]int{"b":2, "a":1, "c":3}, want: `{"a":1,"b":2,"c":3}`}, + { v: map[int64]int{1:-1, -2:2, 0:0}, want: `{"-2":2,"0":0,"1":-1}`}, + { v: map[uint]int{1:-1, ^uint(0):2, 0:0}, want: `{"0":0,"1":-1,"18446744073709551615":2}`}, + { v: map[uintptr]int{uintptr(0xf):0xf, uintptr(0x0):0}, want: `{"0":0,"15":15}`}, + { v: map[encoding.TextMarshaler]interface{}{ + nptext : nil, + &ptext : struct{}{}, + text0 : "", + &text1 : 1, + text2 : text2, + }, + want: `{"":"","1":1,"2":"2","ptrisnil":null,"ptrtokey":{}}`, + }, + } + + for _, tt := range testCases { + out, err := encoder.Encode(tt.v, encoder.SortMapKeys) + require.NoError(t, err) + require.Equal(t, tt.want, string(out)) + } +} + +func TestIssue115_MarshalLargeIntKeyMapWitSort(t *testing.T) { + N := 10000 + m := map[int]string{} + for i := 0; i < N; i++ { + a := strconv.Itoa(i) + m[i] = a + } + + exp, err := json.Marshal(&m) + require.NoError(t, err) + got, err := encoder.Encode(&m, encoder.SortMapKeys) + require.NoError(t, err) + require.Equal(t, string(exp), string(got)) +} + diff --git a/issue_test/issue8_test.go b/issue_test/issue8_test.go index 827a121..ca3fe89 100644 --- a/issue_test/issue8_test.go +++ b/issue_test/issue8_test.go @@ -17,10 +17,11 @@ package issue_test import ( + `encoding/json` + `reflect` + `testing` + . `github.com/bytedance/sonic` - "encoding/json" - "reflect" - "testing" ) func TestFloat(t *testing.T) { diff --git a/issue_test/issue90_test.go b/issue_test/issue90_test.go index 7693b30..49c79b4 100644 --- a/issue_test/issue90_test.go +++ b/issue_test/issue90_test.go @@ -18,9 +18,9 @@ package issue_test import ( . `github.com/bytedance/sonic` - "encoding/json" - "fmt" - "testing" + `encoding/json` + `fmt` + `testing` ) func TestUnmarshalInfinity(t *testing.T) {