From bbae01600f1aae8dbae3c75a4073606901cbf5f5 Mon Sep 17 00:00:00 2001 From: dave caruso Date: Thu, 4 Jul 2024 22:56:20 -0700 Subject: [PATCH] fix bug and try to improve perf (it still slow) --- bench/bun.lockb | Bin 69389 -> 69762 bytes bench/package.json | 1 + bench/snippets/transpiler2.mjs | 12 ++ src/js_parser.zig | 231 ++++++++++++++++----------------- src/js_printer.zig | 3 +- 5 files changed, 125 insertions(+), 122 deletions(-) create mode 100644 bench/snippets/transpiler2.mjs diff --git a/bench/bun.lockb b/bench/bun.lockb index 78eea70b0ef1beb98f2becda86abc2eff95d0c4f..d05e9a3f259e78547bdc2932eed881eba764b067 100755 GIT binary patch delta 9831 zcmeHNdstP~wqI+@1~!5SVzAi0P!v(wz}_Gr$O6++x0snqSeRQy!FVWWiVxUoC*P%x zQppg}Bc*6&pymtT??;+eVwqu&qowCwt8@SR=0-wt?m#NalW6~>u=zC3MsO3~mM zy}$F7CBC*VO>b)$QM+_%im9_flDs9!xnSvmwy7m9G6~WPnPUqJ^0@phfccAVlH>yXi;B0-PONM|j9}CXia{wogOISt2O-&ZL4MYxoV+Y4FDEZ&bi~;F?Z{_a6C9JWBc9JH-ss|$ zkQ|R!A$=ehKyn+UkZfm4cFyQ*_%RO+vQKSWYUOl0gP=pbQfF4tsL46GW2Ci#l7uqO zC6NA*B`%o{$)S7J#qE%2!x<0Bj(33Mdd*z&&Lfh9Kj&AtvHl_?mp=^2c4}R+0I9hv$(~ptx%|tn^8S$A&;4Ds!1#C5dgPjYrBW3#c-mz< ziXz75I>s#nXIJM!@(|a-S8mA1q7A(t8VCXJ0K**V@kLohqtTta&~sOhMrxs`hGa)9 zQCj|Ta2~-T)Z_MsK(>`iCFhsXTEXLxt)Q?$!424Kny1B%+}xa^Nm3ZNH}b<`T(O7b zxrpW+xsEYe!QkwuJ7fUlT~~Q2RFc|&w*==Ia1Wdv5e~;9u^bt2-uZP0ZIE}yX%%vi zkIp;$Lb5}-3EH5x39SisuWRCIE}dpu`OWm$noFh>gIq$2w<`aQGH~xnW!|c(K$avc z>KP~`Fiw7(6dzUojxu~yQw%n>6lj`|!#hqcqk3qrlG03-11SUdepH5gDb?e?pA=tJ z{)sYh??z>~zee@Gs_An#NlMae_{7Pnl+j!@Ej3C~f8@#Jh>Z6H835u&m1rc46hBpd zjxzjI)2r^9V{XE+qf`&gT~hp2qXj!y4=VRfl*_5iUo~B04jTrl^o=vM!tRLR!tD^B zIMZ-2cEE$}oBjya8_X!!Z7{Cq!7Z4&nj{G;3A&JGadHl21gP=`Dhp7JUqPc%YrjM} zj+BqBJ4(vbfKIKUvX-jxHu6#^(U3P@8ji@e)E=c(9}2Z+3dNFl_i`w3E-mB?T{ zH3z9-LR*iNRxTWymsWQ&Qkvy`Kc`Hs^tzcyX}(k=HCR}=hm=;gANFLewQ{7ia^E7A zMpr_UJh3ftP!W#cIJuCNPO7|-GI0N#$~vh=uXd93G?jNsG>-Z?wF{|1BF_tZ(g=~t zMrxQy)%~0bXwMCE-E5==i_|Wp28fh9wmPBp8;%sOTP$>bVo4r!zG)aW_x6-mQ%1Nd zAEvT!)$}#?|m;GmM6J zO*S3H18sn&3XC)Occ#l-ljW(TbW`PXl!5yaD#QH>)pt{k!CfRNpEAP}jq{LtmQ(VV zR2HEcle%JhQfAjg;|!#Fi!$es>cuIO8I5yfwLM@c)mv1#mXt`Gf$<=Zi(_Zs<*2015#|NF&)7j zE_$>TsWIG3Q<6>V5%yOsj~BprQH#YQZ=#GC)ie+TG*nn_Ks(#PlC*ZPM85;mdd;=F z#7fem&>09NCi2&or1Wf;(dF30yUVTa^b9Lbli zBnM)Z^80~yvBbkvY6o~wW$y7+bSh5?4xX@#@iHoIyd{WwmbFIEw<}W07#_q~z zX`sD50_k)QPeu;c!lo~-hb6Z)2w?q_09zU2;+P-e`Yp-zh5>jqOV4p>x{}gX1l++4 z*Z)Lvt78DRmgSOTA^G|>*%JBFh4xovsZjk!f&XK@*8VR%X!jux|I}2KP7pJ!CCDp@v!7UsC4E3nq*h2UHXS5585Vxf+?*g1( z89|YKe!>;`QD$za{JO<#s zQ+glZ2F|(Uc}REQV;8>!$rnpE;8TF>Uj_L3U(07vi9Kv^$uA(;;MV|OEZLzy0bK3| zzz+R~i{Ez1?;-U|Y=j`cZ-xj0Z;l+$-#0_CIby!!`h7Fx-Szj)P)vr#P4WMGGvu}U zZ`llMGJ0?7JRsHIFRFY&+7~5tpLP7<`lS`kKYX_TvupQe+!%4Tc+qEWH+wvK?47^P zY0)O>!m(35yq-}K_p~fsJ2K6@dBG!__fM}UZI4^>2w{ee6n3$N;kk(zTiV?FW6hMk>P+-+rd@uMif5vKCFmd61`01h z|G*ZP*yU=f2b))l{*~J0%{0Fh{d1y!U|T4`iT;6=JMHpTx&gLw7Wy~KF4s`mEc9q2oLQ%w|!mKRmT21RY zbBtbk!4(12X3nr>^SzDN@sioJCS=Y71O2qnQ~4MlwRp^my8PpOENldKf_1cY$_T^K z@tRwUo{^1yk$4+M=J)KtBl_=72diS8l%a_bYzhJtK0ocP*~RZ!4_li(BEmsBO%kdry;INt12X zwSDAfG_f{OeuFmG2Fi=*NNt$UODN0_kQqP{{aE{yJdM(}b&yBU z#BD8n@MR`G9R@=_MN79Omf|N1@m};fBzsT?@O#w-;3B~9UdMqGz)9c~@JC=huo2+5 z(ltOiz^^|yfu4W^7zI2BqyoJF6-Wez0Q_N-#&4Lst@H!>1NS|G zcLyW~o8KF+0FMFulFKi?yMevHXkaA3qZH*DnV-mSM+YN2G_@vSN1&l}3z8fhF2q6Q z>F_o%72wzJ1;A^-8h``$2Ji=9J}?L1_ka?B2ayMJ8c+<(1ZD%X04Jc^c@^n-06#Y| z0ZN52Ho^v81USv$23T@MZjjRp0XDc8SOhEucpmeY$Sc4yfB-JL0$2{L0?L4u!0WDb zqw^@q9#sH5J$OoT1C0eVV{EV*;7F|lDgo{#M`25`SoXU2m$%E)myKQnNnfIj0V<~5k4Zrf0?@`P=VSG`0LPEvG>7j4zLhWda z)q<&goX+o2k;%oBY3^4I9eFV8H2>eM;oN?cH1o~LmhyFT^K43HJ zC$nDV4VPc^_`)XYN3%YZwjM}B6doKQkz8hl@Yq54mVDv43BE-LyD~3x6qSF@1o}qDQ5iyw{`o54*N~*cfyA_Z9D_0;*b*i z&oOE)TP-{}F-DHP}qGpr@Fu1AtgL4iU?q67fvN4($XK3Bwaq>KBdL%7YKjHPdk=|Nev*<2L z*))s#$!YatIhloO*c%2J!xUqQjgeAm@e#$WpS1RJuP@o^F}+2jKAR34QSd_c2}ouXFEVwl-=mQsz;`tREE@ z<%A7c_=9wzQGbV)9#ss@0%_CHAhUkbJoGQ~iZ9=5@=9Z|c)AJ)^pokAPt8q>Tk-kZ zje7m;xdl|!-W@q~eWQLNMITqN zE!dAM=5?^r8dE0qgR6($c&4hjvDAJlJ+8#+N8EV>yL-{z3)`3~twH%|na^`qv;ujTgXa$twHu0(-Z)b^wzzf2F#DrWs;J@`)Etqu0}3v@Hl z)=lJhMv>p6xhMO`C+W<|p=SLwzT=Uqr^3wR-89z|u9_{p`NS&V@t%-g3;lnpKY2cv|(of=`aNCj*;A+k`)q zaykaT`1_p+a2G)p$#x>CpZWKCV(1fTi>Xex6CEWAc|<2*&e@U|tMlG0Pp#Ig+i-2k232b5& zP6vdD80-H92nt6s&yTw`AUc6(WFE?!3)LYtD`BuK!Ri=koXt+DG61w7E4 zMf3aKuU#KHLzEMa>@6CYw}R93&F1H!iZPr55<#7>ii*{y^!Mdw2OY@OF2VQCYc>MUx!)V;s5of25kMi^(0@H1f!Zhm;Z8TNT~@d#9Rpe;Xh852=~!U;qFB delta 9658 zcmeHNdsr0LlJ7Gx$N&TQKo|xDA5lkTU>I;fghumH2TV+2#$?rqfI0?592HR%kQ<}% zi4~*9_*&zQXwW72jN&sI)F{4^XkvWa@7_d=xmRz#tTEY`{Z;q0#~PV__Ns>O!qh&^-4frP?jJF2c*#!Gx{G zdx|EEaTnw-gTTIX$K@2fjQGv~tUnCOG3)|m{bLZgd=q5$zS2uWA+S6YGV2>4aBMzk znrRJW&cqr}_@NS=seB(&!V%YjvhSh^`IFs+`BI^~(48AMX2M34v#*IclgGuqoL{=q zD=z`%d{luNK}$iojR~Oar(~QvcO2pO#Gy<{>6p>d%gL12SE~jMQwDR0X)OcK=ocWi!sd74KC&-;Z&7cFI=gjgn z%qj3ly&&^Qgjpo1E9m##)d;=-W&2dGeM1+ue=qk`6Vd?6J@L-Ea%o5}b-Inq@x+ZO z$QhdsnL|tj<)L+>@7&OCP#*GqXrLqLX*lK#kN4zza*>{mR<(=8@oEa52jxI6fpXXT zf%3?8Kwvz*e@6Wnsa&d9Y*P(Apixji^74$2-?|^4srIf5VWe6NO|6lEs(g#=RHS;69R?abHaqqb#nI+bA14 zW9`a>10O05a)`Oq49#h>1k1vY+_AgZy-pILDG{g zD#afHS8JsN5{dbhp{jx%Vi>vG$zlc7w3GE0pphxAb(-i(mN41y0wz>%SfT^y)MBa$ zll9k7mPv)iG($Y5-xG?~h6blVk|80qHV*xENGVhslxFx29Jj4>&Jd3g=P`>>A{h<| zgH{np#puAhka!s20sd}5N`d4@wh)KtLY4?wpNsjENO5h_49mcAO$=E(2L`J-LKcB! zG0Ea-a^qf2H740`1{0Y*1}gE#kR?(!j0{oBTCrclhmh2yv1G&p!;&9IpdSe7X{Dzt z!ExUL$kxVTxCn{UF43I`ham{7Q-V^5J@kddL+P*dp$XDpNIpt$qcOWV@4iaQ{{o3~ z>!;|dA#rcjEFFi$aqE@57~1|qr|$MZl>^c=1D6S-q${U5PxJV%AG zY5E_)4OX}TSWQ*#4RC{$vTF~@tXO-ql(OmIkXzI}0Z#SOAyVTefqP!jHiA>_BCuGg zb@RZfp0_^Wz6Ymz$%@hy zqj+CUpju0s-rhr!hBJo;zZsgZAZJo-&oq6vo>*HHn;GExGiNvkj&rB3VG>#5W$_ZZ z<7LAJgp>jeCQ@jMKZK_t2wo*bPqHM)Vk)^4WZmI7+MJM~yB|k46EehTve;zZQ7etN zWf<;a^&f;v5*3F!#B0Bt|l8C7VZ6&y=%<1l?QjgR= z9kCMfYn}Hfyc1F`rUJmU$NsZ0HM})mFT7h(N-<+lx;nL6IWw3Rk`I9KQ?yvbB^`iW zgL3^E#fy*Uv2s0(vL5j&{?b%4i7Cg1G>=x8bZ-IHG38>)23VIBnhDBRCcuI92Uw4> zP%fri{y4zp$hvYVlvekNN#*V#`^v?Xt+Q0|SE-tPhjx-?zU{wAul|IbuV zT>ldd{15hMpYvSijrf-|0A+K$<^PwIABlwkKR9nic^i1^AyiOYdU!bxz&fw?VU)w% z?A8C0@_=mtn7#{eH@5=puMXhy_q_5BP`;S5emB7SJ@|qHQJ4pm74HK~_bR3TNZG-D zZ#h$z4*?v&5rD6UQMNk}p&Z~f zfa#wA_V<_JxbgKDfE)f2V1pX~2lQ93eA`R^4ys+eJTiEse_tYzgeL&L6#DxTiA?;4 zmF)K=QkgT-?@OezA{|3E{(miz*cB_+Z(btn_dND)cWNkfiV`wRaFPE6BNa|?ic;DG zX*Z$w&()ImIh<3DQMK=^m$emF9U|G}mLKZy?R1)X6SNnQWxhlbvEdHAA`qDSL`j zETolFT(n|}k$!@-m*BDOQntI{G&q{e!fcOf%5G8R*{(r&vqJAvHpZ zD|3qVR9c4qm7#x-Hjrf|`Zp8(o9PrA=@O)ikkV&4#XB@_7Wy{}{ex6PspaTjIr>-b z6kDhn(hW%26;4r0EAdnLiVE}((l#1ciT+ihf0a&ANB1Co4{5|~r>Liz+34SF^sg!x zAF;(%F`@18D#XU4A{I0Vl&%N1qu3x{LrMPViAC5aE}=tJZT-(e9SU{*ojJL>rLQhy zllqHuDs%?T!5qq&m+Sv2c8!58c9n`NPk!Ol{7I5Do6at1>wokSwMDu!r!k0M#2$P= zpc?xgAh-erE%x`{XH|bggxPH_Y_I)@m|Nhsj`w)q-S+yfOImda*i^4h+@S3=ZhK#K zoxT6Fda>r;0B&kA3%E3y-~U6El9sI$Zx&wkmhrZV>+!~fw_1F0Q(Ts^oh3ZNyHpy` zcZxkRBQ`14LmhSu!nAYmNHfiW?p0teFb{YQm=Bau!;Wr7erL=8_=_f=?(BG045i0+ zcJ9buLdgKW@+5rK@h7D6;V=aB86cSE?M#d2ozYc*18V@z0Ox@7z%jbMvu*hXaBl-O zz-nMEz>6<$dAPwD~m1Udk` zJ-P;5XE%qyaGH2?cM-S*dcM&F%Z3`+(8FO8}2fig!e$y%fBotJq$jygOW1z7-ly1Y2;*cn;J8CBPP7 z9`HJ#C21A-KL9**vw>N_On?WB2d)&D29yDnKm||^XnqJh4?RzTS6Ic)*ad|3kUfe*1b*(!Rmr288{}@WxS?1f0ejQdvOmZ*MYGjmkt~-;ppJf0XyDu#)D(cE?@O-{zyxB&(WFY35*c+-I(m9VLhnt(GO9dc7wf3$h)KOCYv-pagHHT4 zaaH6rU$(VbvD|4VriP-v$8HXJbFWU!O138UwkAoT6#W4Y*pLVJHT3caCbM={`rNj) zXM2TSyP?FTq`!gIeqc)0&QxC%Z+-s6-L(^;@2&P;J4qfo>ekOcEXn!zP?2cmsm{N0 zegn_iNon^xSN`S+zr71JQ`DMO)bXfEETOccCbM?VdeXnS>!86WK7tje48y;XijLaN z+Ij8%XWU10_s=hZp8E>@K595R79;t{v23$;>Kf4Oik8Bucc!Ql5oYb| zvh4Mg;#P$#ueTVqqo+=oL_cz$u!~K!`h-c>(1jL!6cMEz?0!9d#vDVR-fNXAi3wJR zbfpX3f=~72R;qYm`r8fPgr|IWpWI9zRlROJIGN zAA46brgm8EkXbutKTzE5^IPSE+O;&8PA{Fdo3-=z8=GDW`Q#HnUP-(wTq4yXZQAks zqV?|${b^>?Q0VzxgLyZEuAjC?X~*%Si@T&}28N_4dX7cf9Y+aG5#GFRq}(RExnI1x zDZYOH)Yk4v#uZ*?*mXTmbxkHwOn1(i%-RY4sDZhGo#qYwOegH>+5)SeoBo?jC=-H5 zGD4>$&!khwLU^bW>7DB%?u`%sPZuSwr2qf` diff --git a/bench/package.json b/bench/package.json index 761fa8936e..6ea67f5c99 100644 --- a/bench/package.json +++ b/bench/package.json @@ -3,6 +3,7 @@ "dependencies": { "@babel/core": "^7.16.10", "@babel/preset-react": "^7.16.7", + "@babel/standalone": "^7.24.7", "@swc/core": "^1.2.133", "benchmark": "^2.1.4", "braces": "^3.0.2", diff --git a/bench/snippets/transpiler2.mjs b/bench/snippets/transpiler2.mjs new file mode 100644 index 0000000000..3269e1a81c --- /dev/null +++ b/bench/snippets/transpiler2.mjs @@ -0,0 +1,12 @@ +import { bench, run } from "mitata"; +import { join } from "path"; + +const code = require("fs").readFileSync(process.argv[2] || join(import.meta.dir, "../../node_modules/typescript/lib/tsc.js")); + +const transpiler = new Bun.Transpiler({ minify: true }); + +bench("transformSync", () => { + transpiler.transformSync(code); +}); + +await run(); diff --git a/src/js_parser.zig b/src/js_parser.zig index 0ebc050efa..e7279d5509 100644 --- a/src/js_parser.zig +++ b/src/js_parser.zig @@ -1867,10 +1867,10 @@ pub const SideEffects = enum(u1) { }; } - /// This is responsible for simplifying an unused expression that may or may not - /// have side effects into a smaller expression. For example, this is where - /// calls with .can_be_unwrapped_if_unused get unwrapped. A `null` return value - /// implies E.Missing, aka "remove this expression" + /// This is responsible for simplifying an unused expression that may or may + /// not have side effects into a smaller expression. For example, this is + /// where calls with .can_be_unwrapped_if_unused get unwrapped. This returns + /// Expr.missing if the node can be removed completely. /// /// Since this is run on many expressions and, more specifically, binary comma /// expressions, a recursive approach will easily stack overflow with large, @@ -1886,30 +1886,47 @@ pub const SideEffects = enum(u1) { .keep => expr, .remove => Expr.missing, .recursive => { - var state: SimplifyState = p.simplify_state.get(); - defer p.simplify_state.release(&state); + var sfa = std.heap.stackFallback( + @sizeOf(Expr) * 1024 + @sizeOf(SimplifyState.Node) * 256, + p.allocator, + ); + const node_allocator = sfa.get(); + var state: SimplifyState = .{ + .exprs = SimplifyState.ExprList.initCapacity(node_allocator, 1024) catch unreachable, + .nodes = SimplifyState.NodeList.initCapacity(node_allocator, 256) catch unreachable, + .current = undefined, + }; - state.exprs.append(bun.failing_allocator, expr) catch unreachable; - state.current = state.pushDependencies(0, &expr, p.allocator) catch bun.outOfMemory(); + if (Environment.isDebug) { + assert(sfa.fixed_buffer_allocator.end_index == sfa.fixed_buffer_allocator.buffer.len); + } - bun.assert(state.exprs.len > 1); // should be more than one dependency - bun.assert(state.current.data.parent_index == 0); - bun.assert(state.current.children_left > 0); - bun.assert(state.current.offset == 0); + state.exprs.appendAssumeCapacity(expr); + state.current = state.pushDependencies(0, &expr, node_allocator) catch bun.outOfMemory(); + + if (Environment.isDebug) { + bun.assert(state.exprs.items.len > 1); // should have pushed something + bun.assert(state.current.data.parent_index == 0); + bun.assert(state.current.children_left > 0); + bun.assert(state.current.offset == 0); + } while (true) { if (state.current.children_left > 0) { - state.startExpr(p, p.allocator) catch bun.outOfMemory(); + state.startExpr(p, node_allocator) catch bun.outOfMemory(); } else { if (state.finishExpr(p) catch bun.outOfMemory() == .stop) break; } } - bun.assert(state.nodes.len == 0); - bun.assert(state.exprs.len == 1); + if (Environment.isDebug) { + // when stopping there should only be one item remaining + bun.assert(state.nodes.items.len == 0); + bun.assert(state.exprs.items.len == 1); + } - return state.exprs.at(0).*; + return state.exprs.items[0]; }, }; } @@ -1929,6 +1946,7 @@ pub const SideEffects = enum(u1) { /// Depends on the expression type /// - e_object: does this have any spreads? /// - e_array: does this have any spreads? + /// - e_if: is this a second pass to visit the test flag: bool, /// Index of the unvisited root expression parent_index: u31, @@ -1944,62 +1962,28 @@ pub const SideEffects = enum(u1) { } }; - // TODO: benchmark ArrayListUnmanaged here. it probably does better in the general case. - const ExprList = std.SegmentedList(Expr, 128); - const NodeList = std.SegmentedList(Node, 32); - - /// We want to re-use memory from simplifyUnusedExpr across invocations, - /// since freeing it will likely do nothing since the parser uses an arena. - /// However, storing the entire state is useless as the segmented lists - /// take up a lot of extra bytes which will always be unused outside of - /// `simplifyUnusedExpr`. This 'SharedMemory' stores just the reused - /// allocations from SegmentedList. - const SharedMemory = struct { - /// Memory sharing requires an exclusive lock, which is OK because - /// simplifyUnusedExpr is not recursive. - lock: std.debug.SafetyLock = .{}, - exprs: [][*]Expr = &.{}, - nodes: [][*]Node = &.{}, - - /// The `SimplifyState` is not fully initialized - pub fn get(shared_memory: *SharedMemory) callconv(callconv_inline) SimplifyState { - shared_memory.lock.lock(); - return .{ - .exprs = .{ .dynamic_segments = shared_memory.exprs }, - .nodes = .{ .dynamic_segments = shared_memory.nodes }, - .current = undefined, - }; - } - - pub fn release(shared_memory: *SharedMemory, state: *SimplifyState) callconv(callconv_inline) void { - shared_memory.lock.unlock(); - shared_memory.exprs = state.exprs.dynamic_segments; - shared_memory.nodes = state.nodes.dynamic_segments; - - if (Environment.isDebug) - state.* = undefined; - } - }; + const ExprList = std.ArrayListUnmanaged(Expr); + const NodeList = std.ArrayListUnmanaged(Node); const ChildrenSlice = struct { - expr_list: *ExprList, + expr_list: [*]Expr, offset: u32, len: if (Environment.isDebug) u32 else u0 = 0, - pub fn at(slice: ChildrenSlice, n: u32) Expr { + pub fn at(slice: ChildrenSlice, n: u32) callconv(callconv_inline) Expr { if (Environment.isDebug) bun.assert(n < slice.len); - return slice.expr_list.at(slice.offset + n).*; + return slice.expr_list[slice.offset + n]; } - pub fn next(slice: *ChildrenSlice) Expr { + pub fn next(slice: *ChildrenSlice) callconv(callconv_inline) Expr { defer { slice.offset += 1; if (Environment.isDebug) slice.len -= 1; } - return slice.expr_list.at(slice.offset).*; + return slice.expr_list[slice.offset]; } }; @@ -2007,10 +1991,10 @@ pub const SideEffects = enum(u1) { state: *SimplifyState, parent_index: u31, expr: *const Expr, - allocator: Allocator, + node_allocator: Allocator, ) !Node { var flag = false; - const offset = state.exprs.len - 1; + const offset = state.exprs.items.len - 1; const children_count: u32 = switch (expr.data) { else => |tag| Output.panic("SimplifyState.pushDependencies unexpected state: {s}", .{@tagName(tag)}), @@ -2018,27 +2002,25 @@ pub const SideEffects = enum(u1) { bun.assert(call.can_be_unwrapped_if_unused); // if false, visiting is useless bun.assert(call.args.len > 0); // if zero, should have already returned .remove const args = call.args.slice(); - try state.exprs.appendSlice(allocator, args); + try state.exprs.appendSlice(node_allocator, args); break :len @intCast(args.len); }, .e_if => |ternary| len: { - try state.exprs.appendSlice(allocator, &.{ - // TODO: always visiting this is wrong and slow - ternary.test_, + try state.exprs.appendSlice(node_allocator, &.{ ternary.yes, ternary.no, }); - break :len 3; + break :len 2; }, .e_unary => |unary| len: { - try state.exprs.append(allocator, unary.value); + try state.exprs.append(node_allocator, unary.value); break :len 1; }, .e_binary => |bin| len: { - try state.exprs.appendSlice(allocator, &.{ + try state.exprs.appendSlice(node_allocator, &.{ bin.left, bin.right, }); @@ -2051,7 +2033,7 @@ pub const SideEffects = enum(u1) { for (items) |prop| { // Spread properties must always be evaluated if (prop.data != .e_spread) { - try state.exprs.append(allocator, prop); + try state.exprs.append(node_allocator, prop); count += 1; } else { // Mark there to be a spread @@ -2066,7 +2048,7 @@ pub const SideEffects = enum(u1) { const properties = obj.properties.slice(); for (properties) |prop| { if (prop.kind != .spread) { - try state.exprs.append(allocator, prop.value.?); + try state.exprs.append(node_allocator, prop.value.?); count += 1; } else { // Mark there to be a spread @@ -2087,11 +2069,11 @@ pub const SideEffects = enum(u1) { }; } - fn startExpr(state: *SimplifyState, p: anytype, allocator: Allocator) callconv(callconv_inline) !void { + fn startExpr(state: *SimplifyState, p: anytype, node_allocator: Allocator) callconv(callconv_inline) !void { bun.assert(state.current.children_left > 0); // already visited parent index const i = state.current.offset + state.current.children_left; - const expr = state.exprs.at(i); + const expr = &state.exprs.items[i]; state.current.children_left -= 1; @@ -2103,8 +2085,8 @@ pub const SideEffects = enum(u1) { }, .recursive => { // push a new `node` - try state.nodes.append(allocator, state.current); - state.current = try state.pushDependencies(@intCast(i), expr, allocator); + try state.nodes.append(node_allocator, state.current); + state.current = try state.pushDependencies(@intCast(i), expr, node_allocator); }, } } @@ -2113,28 +2095,35 @@ pub const SideEffects = enum(u1) { fn finishExpr(state: *SimplifyState, p: anytype) callconv(callconv_inline) !FinishResult { bun.assert(state.current.children_left == 0); // already visited - const expr = state.exprs.at(state.current.data.parent_index); + const expr = &state.exprs.items[state.current.data.parent_index]; var children: ChildrenSlice = .{ - .expr_list = &state.exprs, + .expr_list = state.exprs.items.ptr, .offset = state.current.offset + 1, .len = state.current.children_count_debug, }; - expr.* = finishExprWithChildren( + expr.* = state.finishExprWithChildren( p, expr, - state.current.data.flag, &children, - ); - state.exprs.len = state.current.offset + 1; - state.current = state.nodes.pop() orelse return .stop; + ) orelse return .@"continue"; + + state.exprs.items.len = state.current.offset + 1; + state.current = state.nodes.popOrNull() orelse + return .stop; + return .@"continue"; } /// `children` in this context is the visited children, which were /// pushed in `pushDependencies`. The exact contents and length are /// dependant on what was pushed. - fn finishExprWithChildren(p: anytype, original: *Expr, flag: bool, children: *ChildrenSlice) callconv(callconv_inline) Expr { + fn finishExprWithChildren( + state: *SimplifyState, + p: anytype, + original: *Expr, + children: *ChildrenSlice, + ) callconv(callconv_inline) ?Expr { if (Environment.isDebug) { bun.assert(nonRecursive(p, original) == .recursive); bun.assert(p.options.features.dead_code_elimination); @@ -2173,7 +2162,8 @@ pub const SideEffects = enum(u1) { .bin_loose_ne, => { if (Environment.isDebug) - bun.assert(isPrimitiveWithSideEffects(bin.left.data) and isPrimitiveWithSideEffects(bin.right.data)); + bun.assert(isPrimitiveWithSideEffects(bin.left.data) and + isPrimitiveWithSideEffects(bin.right.data)); return Expr.joinWithComma(left, right); }, @@ -2187,6 +2177,7 @@ pub const SideEffects = enum(u1) { // Preserve short-circuit behavior: the left expression is only unused if // the right expression can be completely removed. Otherwise, the left // expression is important for the branch. + // TODO: audit this if (bin.right.isMissing()) return left; @@ -2203,33 +2194,43 @@ pub const SideEffects = enum(u1) { .e_unary => return children.at(0), .e_if => |ternary| { - ternary.yes = children.at(1); - ternary.no = children.at(2); + if (!state.current.data.flag) { + ternary.yes = children.at(0); + ternary.no = children.at(1); - // "foo() ? 1 : 2" => "foo()" - if (ternary.yes.isMissing() and ternary.no.isMissing()) { + // "foo() ? 1 : 2" => "foo()" + if (ternary.yes.isMissing() and ternary.no.isMissing()) { + state.exprs.items.len -= 2; + state.exprs.appendAssumeCapacity(ternary.test_); + state.current.data.flag = true; + state.current.children_left = 1; + state.current.children_count_debug = if (Environment.isDebug) 1 else 0; + return null; + } + + // "foo() ? 1 : bar()" => "foo() || bar()" + if (ternary.yes.isMissing()) { + return Expr.joinWithLeftAssociativeOp( + .bin_logical_or, + ternary.test_, + ternary.no, + ); + } + + // "foo() ? bar() : 2" => "foo() && bar()" + if (ternary.no.isMissing()) { + return Expr.joinWithLeftAssociativeOp( + .bin_logical_and, + ternary.test_, + ternary.yes, + ); + } + + return original.*; + } else { + // "foo() ? 1 : 2" => "foo()" return children.at(0); } - - // "foo() ? 1 : bar()" => "foo() || bar()" - if (ternary.yes.isMissing()) { - return Expr.joinWithLeftAssociativeOp( - .bin_logical_or, - ternary.test_, - ternary.no, - ); - } - - // "foo() ? bar() : 2" => "foo() && bar()" - if (ternary.no.isMissing()) { - return Expr.joinWithLeftAssociativeOp( - .bin_logical_and, - ternary.test_, - ternary.yes, - ); - } - - return original.*; }, .e_object => |obj| { @@ -2237,7 +2238,7 @@ pub const SideEffects = enum(u1) { // "..." triggers code evaluation via getters. In that case, just trim // the other items instead and leave the object expression there. var properties_slice = obj.properties.slice(); - if (flag) { + if (state.current.data.flag) { var end: usize = 0; // Spread properties must always be evaluated @@ -2286,7 +2287,7 @@ pub const SideEffects = enum(u1) { .e_array => |array| { var items = array.items.slice(); - if (flag) { + if (state.current.data.flag) { var end: usize = 0; for (items) |item| { @@ -2357,10 +2358,6 @@ pub const SideEffects = enum(u1) { .keep, .e_identifier => |ident| { - std.debug.print("awa {} - {s}\n", .{ - ident, - p.symbols.items[ident.ref.innerIndex()].original_name, - }); if (ident.must_keep_due_to_with_stmt) return .keep; @@ -5567,8 +5564,6 @@ fn NewParser_( // If this is true, then all top-level statements are wrapped in a try/catch will_wrap_module_in_try_catch_for_using: bool = false, - simplify_state: SideEffects.SimplifyState.SharedMemory = .{}, - const RecentlyVisitedTSNamespace = struct { ref: Ref = Ref.None, data: ?*js_ast.TSNamespaceMemberMap = null, @@ -19792,8 +19787,6 @@ fn NewParser_( } }, .s_expr => |data| { - const should_trim_primitive = p.options.features.dead_code_elimination and - (p.options.features.minify_syntax and data.value.isPrimitiveLiteral()); p.stmt_expr_value = data.value.data; defer p.stmt_expr_value = .{ .e_missing = .{} }; @@ -19805,14 +19798,10 @@ fn NewParser_( p.commonjs_named_exports_needs_conversion; } - data.value = p.visitExpr(data.value); - - if (should_trim_primitive and data.value.isPrimitiveLiteral()) { - return; - } + const visited = p.visitExpr(data.value); // simplify unused - data.value = SideEffects.simplifyUnusedExpr(p, data.value).toOptional() orelse + data.value = SideEffects.simplifyUnusedExpr(p, visited).toOptional() orelse return; if (comptime FeatureFlags.unwrap_commonjs_to_esm) { diff --git a/src/js_printer.zig b/src/js_printer.zig index 783ee43014..fd7c4e32d4 100644 --- a/src/js_printer.zig +++ b/src/js_printer.zig @@ -2296,7 +2296,8 @@ fn NewPrinter( .e_missing => { if (bun.Environment.isDebug and !p.debug_allowed_to_print_missing) { // e_missing is allowed in arrays, so this cannot universally panic - Output.panic("Attempt to print .e_missing outside of an array", .{}); + Output.debugWarn("Attempt to print .e_missing outside of an array! Check bundle for 'e_missing'", .{}); + p.print("()"); } }, .e_undefined => {