Sort arbitrary variants deterministically regardless of content order (#10016)

* Fix off-by-one error in variant sort mapping

This didn’t actually have any negative effect because of how it was implemented. But it only happened to work right :D

* Make arbitrary variant sorting deterministic

* Update changelog

* Use faster byte-order comparison

We really only care that the order is _always_ the same. localeCompare is still locale dependent based on environment. This ensures it’s dependent on content only.

* Fix CS
This commit is contained in:
Jordan Pittman 2022-12-08 09:05:46 -05:00 • committed by GitHub
parent fde30c307e
commit cea08380dc
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 220 additions and 20 deletions

View file

@ -1,6 +1,7 @@
// @ts-check
import bigSign from '../util/bigSign'
import { remapBitfield } from './remap-bitfield.js'
/**
* @typedef {'base' | 'defaults' | 'components' | 'utilities' | 'variants' | 'user'} Layer
@ -190,7 +191,7 @@ export class Offsets {
return {
...this.create('variants'),
variants: 1n << this.reservedVariantBits,
variants: this.variantOffsets.get(variant),
}
}
@ -243,12 +244,68 @@ export class Offsets {
return a.index - b.index
}
/**
* Arbitrary variants are recorded in the order they're encountered.
* This means that the order is not stable between environments and sets of content files.
*
* In order to make the order stable, we need to remap the arbitrary variant offsets to
* be in alphabetical order starting from the offset of the first arbitrary variant.
*/
recalculateVariantOffsets() {
// Sort the variants by their name
let variants = Array.from(this.variantOffsets.entries())
.filter(([v]) => v.startsWith('['))
.sort(([a], [z]) => fastCompare(a, z))
// Sort the list of offsets
// This is not necessarily a discrete range of numbers which is why
// we're using sort instead of creating a range from min/max
let newOffsets = variants.map(([, offset]) => offset).sort((a, z) => bigSign(a - z))
// Create a map from the old offsets to the new offsets in the new sort order
/** @type {[bigint, bigint][]} */
let mapping = variants.map(([, oldOffset], i) => [oldOffset, newOffsets[i]])
// Remove any variants that will not move letting us skip
// remapping if everything happens to be in order
return mapping.filter(([a, z]) => a !== z)
}
/**
* @template T
* @param {[RuleOffset, T][]} list
* @returns {[RuleOffset, T][]}
*/
remapArbitraryVariantOffsets(list) {
let mapping = this.recalculateVariantOffsets()
// No arbitrary variants? Nothing to do.
// Everyhing already in order? Nothing to do.
if (mapping.length === 0) {
return list
}
// Remap every variant offset in the list
return list.map((item) => {
let [offset, rule] = item
offset = {
...offset,
variants: remapBitfield(offset.variants, mapping),
}
return [offset, rule]
})
}
/**
* @template T
* @param {[RuleOffset, T][]} list
* @returns {[RuleOffset, T][]}
*/
sort(list) {
list = this.remapArbitraryVariantOffsets(list)
return list.sort(([a], [b]) => bigSign(this.compare(a, b)))
}
}
@ -268,3 +325,27 @@ function max(nums) {
return max
}
/**
* A fast ASCII order string comparison function.
*
* Using `.sort()` without a custom compare function is faster
* But you can only use that if you're sorting an array of
* only strings. If you're sorting strings inside objects
* or arrays, you need must use a custom compare function.
*
* @param {string} a
* @param {string} b
*/
function fastCompare(a, b) {
let aLen = a.length
let bLen = b.length
let minLen = aLen < bLen ? aLen : bLen
for (let i = 0; i < minLen; i++) {
let cmp = a.charCodeAt(i) - b.charCodeAt(i)
if (cmp !== 0) return cmp
}
return aLen - bLen
}

82
src/lib/remap-bitfield.js Normal file
View file

@ -0,0 +1,82 @@
// @ts-check
/**
* We must remap all the old bits to new bits for each set variant
* Only arbitrary variants are considered as those are the only
* ones that need to be re-sorted at this time
*
* An iterated process that removes and sets individual bits simultaneously
* will not work because we may have a new bit that is also a later old bit
* This means that we would be removing a previously set bit which we don't
* want to do
*
* For example (assume `bN` = `1<<N`)
* Given the "total" mapping `[[b1, b3], [b2, b4], [b3, b1], [b4, b2]]`
* The mapping is "total" because:
* 1. Every input and output is accounted for
* 2. All combinations are unique
* 3. No one input maps to multiple outputs and vice versa
* And, given an offset with all bits set:
* V = b1 | b2 | b3 | b4
*
* Let's explore the issue with removing and setting bits simultaneously:
* V & ~b1 | b3 = b2 | b3 | b4
* V & ~b2 | b4 = b3 | b4
* V & ~b3 | b1 = b1 | b4
* V & ~b4 | b2 = b1 | b2
*
* As you can see, we end up with the wrong result.
* This is because we're removing a bit that was previously set.
* And, thus the final result is missing b3 and b4.
*
* Now, let's explore the issue with removing the bits first:
* V & ~b1 = b2 | b3 | b4
* V & ~b2 = b3 | b4
* V & ~b3 = b4
* V & ~b4 = 0
*
* And then setting the bits:
* V | b3 = b3
* V | b4 = b3 | b4
* V | b1 = b1 | b3 | b4
* V | b2 = b1 | b2 | b3 | b4
*
* We get the correct result because we're not removing any bits that were
* previously set thus properly remapping the bits to the new order
*
* To collect this into a single operation that can be done simultaneously
* we must first create a mask for the old bits that are set and a mask for
* the new bits that are set. Then we can remove the old bits and set the new
* bits simultaneously in a "single" operation like so:
* OldMask = b1 | b2 | b3 | b4
* NewMask = b3 | b4 | b1 | b2
*
* So this:
* V & ~oldMask | newMask
*
* Expands to this:
* V & ~b1 & ~b2 & ~b3 & ~b4 | b3 | b4 | b1 | b2
*
* Which becomes this:
* b1 | b2 | b3 | b4
*
* Which is the correct result!
*
* @param {bigint} num
* @param {[bigint, bigint][]} mapping
*/
export function remapBitfield(num, mapping) {
// Create masks for the old and new bits that are set
let oldMask = 0n
let newMask = 0n
for (let [oldBit, newBit] of mapping) {
if (num & oldBit) {
oldMask = oldMask | oldBit
newMask = newMask | newBit
}
}
// Remove all old bits
// Set all new bits
return (num & ~oldMask) | newMask
}