Speeds up the emoji search by 93%. #215

transbitch · 2023-10-15T18:27:10Z

transbitch commented

2023-10-15 18:27:10 +00:00

Improvements:

Startup:
6.69s -> 458.10ms (93.15% speedup)

Search:
135.69ms -> 98.81ms (27.18% speedup)

Should be nearly identical client-side.

Improvements: ``` Startup: 6.69s -> 458.10ms (93.15% speedup) Search: 135.69ms -> 98.81ms (27.18% speedup) ``` Should be nearly identical client-side.

transbitch added 2 commits 2023-10-15 18:27:12 +00:00

da63b2841d Started rewrite

a9adaf6d2a Wrote the fucking thing

Aevann commented

2023-10-15 21:15:45 +00:00

unreadable diff, too many pointless changes to review

Aevann closed this pull request

2023-10-15 21:15:45 +00:00

transbitch commented

2023-10-15 21:30:17 +00:00

Poster

I accidentally used autoformat on one file. Other than that -- ignore the diff changes on emoji_modal.js, I deleted the file and started over. It's really not that much tbh. Added a special attribute, data-emojis, on the html of the elements which u can type emojis in, which accounts for some other changes

Aevann commented

2023-10-16 12:20:02 +00:00

revert all unnecessary changes so i can actually review this

Aevann reopened this pull request

2023-10-16 12:20:02 +00:00

Aevann changed title from ~~Speeds up the emoji search by 93%.~~ to Speeds up the emoji search by 93%.

2023-10-16 13:32:48 +00:00

HeyMoon commented

2023-10-16 14:03:58 +00:00

Note that I have not actually run the code yet. That is pending me more having more time. Also I know fuck all about javascript. I mean technically I worked with it like five years ago but that was back when orangeman was in office. So I will leave reviewing the main logic to someone that knows JS. Maybe Doctor Transmisia or Nekobit, the original authors.

If possible, @transbitch, could you briefly summarize the advantages/disadvantages of your approach/Dr T's approach, from a technical perspective? Like, why did you need to rewrite emoji_modal? Would it be possible to make changes to this file that weren't as invasive, or is a rewrite of the caliber that you are engaging in required?

I ask because this file has been modified quite a bit by @Aevann, probably in response to bugs he encountered in managing the site, which you may not be privy to. Therefore, a major rewrite could undo the work that @Aevann has put into the file and degrade user experience.

Was it necessary to change the names of the CSS classes in main.css? If not, revert pls, will improve readability
Please undo the formatting changes in core.js
For cursormarsey.js, what is the advantage of not dumping everything into global scope? I am not a js-cel, so I am not entirely sure. I am worried that this will break functionality.
Very funny but deleting the original MIT clause ruins the joke for future code spelunkers. Also why did you change the license. Please explain like I am a corporate dev that only makes proprietary software (not me)

/*
This program is free software: you can redistribute it and/or modify
it under the terms of the GNU Affero General Public License as
published by the Free Software Foundation, either version 3 of the
License, or (at your option) any later version.
This program is distributed in the hope that it will be useful,
but WITHOUT ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
GNU Affero General Public License for more details.
You should have received a copy of the GNU Affero General Public License
along with this program. If not, see <https://www.gnu.org/licenses/>.
// This code isn't for feeble minds, you might not understand it, Dr. Transmisia.
// Lappland, you are an absolute idiot and an embarrassment to the Rhodesian people.
// I have done the very thing that you decried impractical.
// The dainty hands of a trans goddess wrote this code. Watch the way her fingers
// dance across the keyboard and learn.

Copyright (C) 2022 Dr Steven Transmisia, anti-evil engineer,
				2022 Nekobit, king autist
*/
// MIT License. Written by @transbitch

Note that I have not actually run the code yet. That is pending me more having more time. Also I know fuck all about javascript. I mean technically I worked with it like five years ago but that was back when orangeman was in office. So I will leave reviewing the main logic to someone that knows JS. Maybe Doctor Transmisia or Nekobit, the original authors. If possible, @transbitch, could you briefly summarize the advantages/disadvantages of your approach/Dr T's approach, from a technical perspective? Like, why did you need to rewrite emoji_modal? Would it be possible to make changes to this file that weren't as invasive, or is a rewrite of the caliber that you are engaging in required? [I ask because this file has been modified quite a bit](https://fsdfsd.net/rDrama/rDrama/commits/branch/master/files/assets/js/emoji_modal.js) by @Aevann, probably in response to bugs he encountered in managing the site, which you may not be privy to. Therefore, a major rewrite could undo the work that @Aevann has put into the file and degrade user experience. 1. Was it necessary to change the names of the CSS classes in main.css? If not, revert pls, will improve readability 2. Please undo the formatting changes in core.js 3. For cursormarsey.js, what is the advantage of not dumping everything into global scope? I am not a js-cel, so I am not entirely sure. I am worried that this will break functionality. 4. Very funny but deleting the original MIT clause ruins the joke for future code spelunkers. Also why did you change the license. Please explain like I am a corporate dev that only makes proprietary software (not me) ``` /* This program is free software: you can redistribute it and/or modify it under the terms of the GNU Affero General Public License as published by the Free Software Foundation, either version 3 of the License, or (at your option) any later version. This program is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU Affero General Public License for more details. You should have received a copy of the GNU Affero General Public License along with this program. If not, see <https://www.gnu.org/licenses/>. // This code isn't for feeble minds, you might not understand it, Dr. Transmisia. // Lappland, you are an absolute idiot and an embarrassment to the Rhodesian people. // I have done the very thing that you decried impractical. // The dainty hands of a trans goddess wrote this code. Watch the way her fingers // dance across the keyboard and learn. Copyright (C) 2022 Dr Steven Transmisia, anti-evil engineer, 2022 Nekobit, king autist */ // MIT License. Written by @transbitch ```

transbitch commented

2023-10-16 15:07:52 +00:00

Poster

Was it necessary to change the names of the CSS classes in main.css? If not, revert pls, will improve readability

Not strictly necessary, but the thing called "speed-emoji-modal" IS NOT A MODAL, so the original class name is a misnomer. It's a dropdown, and "quick" makes more sense to me than "speed", so while we're at it might as well change it to "quick emoji dropdown". If it really ruffles some feathers then I can change it back.

Please undo the formatting changes in core.js

OK

For cursormarsey.js, what is the advantage of not dumping everything into global scope? I am not a js-cel, so I am not entirely sure. I am worried that this will break functionality.

It conflicted with stuff while I was writing, so I put it in its own scope to resolve the error. It doesn't conflict anymore, but putting it in its own scope is good practice, so I left it in. I can revert this if required. This file needs some work anyways; using top and left can cause the browser to unnecessarily reflow the page. It should use transform, which enables GPU acceleration, and should maybe use a spritesheet/data URIs.

Very funny but deleting the original MIT clause ruins the joke for future code spelunkers. Also why did you change the license. Please explain like I am a corporate dev that only makes proprietary software (not me)

Wdym the original MIT clause? Legally speaking, no licenses "changed," the old code still has its GNU Affero license and with the new code I wrote (not derived from the old code), I opted to license it under MIT. GNU Affero is a copyleft license, meaning that any derivative work needs to have the GPL license. It is legally a PITA. MIT has no such restriction, it is essentially a "do whatever" license. You can create a derivative project from MIT code and license it as GNU Affero, but you can't create a derivative project from GNU Affero and license it under MIT. So in other words, MIT is always compatible with GNU Affero, but Affero isn't always compatible with MIT. If you would like, I can offer both MIT and GNU Affero licenses, but again this is kind of pointless since MIT is compatible with everything. However, I understand why you would want to do this for conformity purposes in the codebase.

> Was it necessary to change the names of the CSS classes in main.css? If not, revert pls, will improve readability Not strictly necessary, but the thing called "speed-emoji-modal" IS NOT A MODAL, so the original class name is a misnomer. It's a dropdown, and "quick" makes more sense to me than "speed", so while we're at it might as well change it to "quick emoji dropdown". If it really ruffles some feathers then I can change it back. > Please undo the formatting changes in core.js OK > For cursormarsey.js, what is the advantage of not dumping everything into global scope? I am not a js-cel, so I am not entirely sure. I am worried that this will break functionality. It conflicted with stuff while I was writing, so I put it in its own scope to resolve the error. It doesn't conflict anymore, but putting it in its own scope is good practice, so I left it in. I can revert this if required. This file needs some work anyways; using `top` and `left` can cause the browser to unnecessarily reflow the page. It should use `transform`, which enables GPU acceleration, and should maybe use a spritesheet/data URIs. > Very funny but deleting the original MIT clause ruins the joke for future code spelunkers. Also why did you change the license. Please explain like I am a corporate dev that only makes proprietary software (not me) Wdym the original MIT clause? Legally speaking, no licenses "changed," the old code still has its GNU Affero license and with the new code I wrote (not derived from the old code), I opted to license it under MIT. GNU Affero is a copyleft license, meaning that any derivative work needs to have the GPL license. It is legally a PITA. MIT has no such restriction, it is essentially a "do whatever" license. You can create a derivative project from MIT code and license it as GNU Affero, but you can't create a derivative project from GNU Affero and license it under MIT. So in other words, MIT is always compatible with GNU Affero, but Affero isn't always compatible with MIT. If you would like, I can offer both MIT and GNU Affero licenses, but again this is kind of pointless since MIT is compatible with everything. However, I understand why you would want to do this for conformity purposes in the codebase.

HeyMoon commented

2023-10-16 16:30:36 +00:00

However, I understand why you would want to do this for conformity purposes in the codebase.

If there's one thing I love, it's conformity. ⬜

anyways, these cleanup changes that you are proposing may be better suited for a different PR without logic, so the scope of the changes is smaller

> However, I understand why you would want to do this for conformity purposes in the codebase. If there's one thing I love, it's conformity. ⬜ anyways, these cleanup changes that you are proposing may be better suited for a different PR without logic, so the scope of the changes is smaller

transbitch added 1 commit 2023-10-25 00:58:14 +00:00

620a6b5f1a Undid some stuff due to rodent bitching

transbitch added 1 commit 2023-10-25 01:14:33 +00:00

ac3d9a337d Merge branch 'master' into emoji-search-fix

transbitch added 1 commit 2023-10-25 01:25:07 +00:00

e5ea709a0e Fixed rodent sneeding

transbitch added 1 commit 2023-10-25 03:07:48 +00:00

0256deda29 capy cant sneed

Aevann commented

2023-10-27 09:31:30 +00:00

too many pointless changes, unreviewable

Aevann closed this pull request