Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: fix the way to get the tmp element (#115) #116

Merged
merged 2 commits into from
Aug 6, 2024

Conversation

nekocode
Copy link
Contributor

@nekocode nekocode commented Jul 30, 2024

从 dom 树中尝试获取之前生成的临时元素,避免元素被第三方卸载后报错

src/color/torgb.ts Outdated Show resolved Hide resolved
src/color/torgb.ts Outdated Show resolved Hide resolved
@hustcc
Copy link
Member

hustcc commented Aug 1, 2024

另外一个点,很重要,torbg 这个方法在上层库中是一个高频操作,之前直接缓存到变量上,性能是最优的,如果每次都去 select 查询一下,性能会有大幅下降,可以使用工具测试一下时间。

如果下降幅度很大,建议看看有没有其他方式去解决。比如在出现异常的地方,catch 一下,然后再去创建。

@nekocode
Copy link
Contributor Author

nekocode commented Aug 1, 2024

@hustcc 你看看这个可以接受不?1270 个元素,使用 getElementById 查询 100 万次耗时 23ms 左右。但是我的机器是 Apple M1 Max 32GB 的,可能耗时会少一些。
image

你说的 catch 的方式性能肯定更好一些,你看下你们比较倾向哪种方式呢?

@hustcc
Copy link
Member

hustcc commented Aug 1, 2024

@hustcc 你看看这个可以接受不?1270 个元素,使用 getElementById 查询 100 万次耗时 23ms 左右。但是我的机器是 Apple M1 Max 32GB 的,可能耗时会少一些。

image

你说的 catch 的方式性能肯定更好一些,你看下你们比较倾向哪种方式呢?

要对比,这么做之后,和没这么做之前的性能。

@nekocode
Copy link
Contributor Author

nekocode commented Aug 1, 2024

要对比,这么做之后,和没这么做之前的性能。

@hustcc 你可以在浏览器 console 上分别用下面这两段代码测试下:

原方式:

function toHex(value) {
  const x16Value = Math.round(value).toString(16);
  return x16Value.length === 1 ? `0${x16Value}` : x16Value;
}

function arr2rgb(arr) {
  return `#${toHex(arr[0])}${toHex(arr[1])}${toHex(arr[2])}`;
}

const RGB_REG = /rgba?\(([\s.,0-9]+)\)/;

function createTmp() {
  const i = document.createElement('i');
  i.title = 'Web Colour Picker';
  i.style.display = 'none';
  document.body.appendChild(i);
  return i;
}

let iEl;

function toRGBString(color) {
  if (color[0] === '#' && color.length === 7) {
    return color;
  }
  if (!iEl) {
    iEl = createTmp();
  }

  iEl.style.color = color;
  let rst = document.defaultView.getComputedStyle(iEl, '').getPropertyValue('color');
  const matches = RGB_REG.exec(rst);
  const cArray = matches[1].split(/\s*,\s*/).map((s) => Number(s));
  rst = arr2rgb(cArray);
  return rst;
}

test = "global variable"
console.time(test);
for (let i = 0; i < 100_0000; i++) toRGBString("black");
console.timeEnd(test);

改成使用 getElementById:

function toHex(value) {
  const x16Value = Math.round(value).toString(16);
  return x16Value.length === 1 ? `0${x16Value}` : x16Value;
}

function arr2rgb(arr) {
  return `#${toHex(arr[0])}${toHex(arr[1])}${toHex(arr[2])}`;
}

const RGB_REG = /rgba?\(([\s.,0-9]+)\)/;

function getTmp() {
  let i = document.getElementById('antv-web-colour-picker');
  if (i) return i;
  i = document.createElement('i');
  i.id = 'antv-web-colour-picker';
  i.title = 'Web Colour Picker';
  i.style.display = 'none';
  document.body.appendChild(i);
  return i;
}

function toRGBString(color) {
  if (color[0] === '#' && color.length === 7) {
    return color;
  }
  const iEl = getTmp();
  iEl.style.color = color;
  let rst = document.defaultView.getComputedStyle(iEl, '').getPropertyValue('color');
  const matches = RGB_REG.exec(rst);
  const cArray = matches[1].split(/\s*,\s*/).map((s) => Number(s));
  rst = arr2rgb(cArray);
  return rst;
}

test = "getElementById"
console.time(test);
for (let i = 0; i < 100_0000; i++) toRGBString("black");
console.timeEnd(test);

测试结果:

在我本地的测试结果是:

global variable: 1034.404052734375 ms
getElementById: 1033.176025390625 ms

耗时几乎一样。而且我看导出的函数还有用 memoize 做缓存优化,感觉对性能应该没太大的影响?

@hustcc
Copy link
Member

hustcc commented Aug 2, 2024

那没问题了~ 👍🏻

@hustcc
Copy link
Member

hustcc commented Aug 2, 2024

单侧报错了,帮忙把 https://github.com/antvis/util/blob/master/.github/workflows/build.yml#L15 改成 16 或者 18 试试看。

@nekocode
Copy link
Contributor Author

nekocode commented Aug 2, 2024

单侧报错了,帮忙把 https://github.com/antvis/util/blob/master/.github/workflows/build.yml#L15 改成 16 或者 18 试试看。

已改成 16 了

@hustcc
Copy link
Member

hustcc commented Aug 5, 2024

image

好像还是报错,看看这个。这个 PR 弄了好久了,好事多磨~~

@nekocode
Copy link
Contributor Author

nekocode commented Aug 5, 2024

@hustcc 嗯,已改。刚在 fork 的仓库上跑了遍,现在应该能通过了:
image

@hustcc hustcc merged commit b00dbcf into antvis:master Aug 6, 2024
2 checks passed
@hustcc
Copy link
Member

hustcc commented Aug 6, 2024

已经合并,如果有兴趣,欢迎加入群聊。

image

@hustcc
Copy link
Member

hustcc commented Aug 7, 2024

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

toRGBString 在特殊情况下会抛出异常
2 participants